[PATCH/RFC] Assorted small changes to runstatus

[PATCH/RFC] Assorted small changes to runstatus

[Juergen Ruehle]

As a regular user of git-status I'd like to propose some small changes and a
fix:

 - wording changes to the section headers

 - more context awareness in the nothing-to-commit line

 - fix runstatus breakage when amending a root commit

Additional Notes:

 - if git-reset <commit> <path> provided the ability to remove path from the
   index (if not present in commit) runstatus could advertise reset as the
   universal unstaging tool (in the non-initial-commit case)

   (this is similar to the recent request on the list to allow
    git-commit -o <path> to commit removal of path)

 - IMO git-rm without -f should not remove anything if HEAD doesn't exist
   (i.e. from the initial commit); this might be a bug

[PATCH 1/4] Clarify syntax and role of git-add in status output

[Juergen Ruehle]

From: =?iso-8859-1?q?J=FCrgen_R=FChle?= <j-r@online.de>

This uses the actual (simplified) synopsis line from the git-add man page and
advertises its incremental nature.

Signed-off-by: Jürgen Rühle <j-r@online.de>
---
 wt-status.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index db42738..34be91b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -15,7 +15,7 @@ static char wt_status_colors[][COLOR_MAXLEN] = {
 	"\033[31m", /* WT_STATUS_CHANGED: red */
 	"\033[31m", /* WT_STATUS_UNTRACKED: red */
 };
-static const char* use_add_msg = "use \"git add file1 file2\" to include for commit";
+static const char* use_add_msg = "use \"git add <file>...\" to incrementally add content to commit";
 
 static int parse_status_slot(const char *var, int offset)
 {
-- 
1.5.0.rc0.g31e2

[PATCH 2/4] Improve cached content header of status output

[Juergen Ruehle]

From: =?iso-8859-1?q?J=FCrgen_R=FChle?= <j-r@online.de>

This tries to be more to the point while also including a pointer on how to
unstage changes from the index.

Since this header is printed in two different code paths and the name of the
reference commit is needed for the unstage part, provide a new printing
function.

Signed-off-by: Jürgen Rühle <j-r@online.de>
---
 wt-status.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 34be91b..98ca135 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -53,6 +53,18 @@ void wt_status_prepare(struct wt_status *s)
 	s->untracked = 0;
 }
 
+static void wt_status_print_cached_header(const char *reference)
+{
+	const char *c = color(WT_STATUS_HEADER);
+	color_printf_ln(c, "# Cached changes to be committed:");
+	if (reference) {
+		color_printf_ln(c, "#   (use \"git reset %s <file>...\" and \"git rm --cached <file>...\" to unstage)", reference);
+	} else {
+		color_printf_ln(c, "#   (use \"git rm --cached <file>...\" to unstage)");
+	}
+	color_printf_ln(c, "#");
+}
+
 static void wt_status_print_header(const char *main, const char *sub)
 {
 	const char *c = color(WT_STATUS_HEADER);
@@ -147,8 +159,7 @@ static void wt_status_print_updated_cb(struct diff_queue_struct *q,
 		if (q->queue[i]->status == 'U')
 			continue;
 		if (!shown_header) {
-			wt_status_print_header("Added but not yet committed",
-					"will commit");
+			wt_status_print_cached_header(s->reference);
 			s->commitable = 1;
 			shown_header = 1;
 		}
@@ -179,8 +190,7 @@ void wt_status_print_initial(struct wt_status *s)
 	read_cache();
 	if (active_nr) {
 		s->commitable = 1;
-		wt_status_print_header("Added but not yet committed",
-				"will commit");
+		wt_status_print_cached_header(NULL);
 	}
 	for (i = 0; i < active_nr; i++) {
 		color_printf(color(WT_STATUS_HEADER), "#\t");
-- 
1.5.0.rc0.g31e2

Re: [PATCH 2/4] Improve cached content header of status output

[Andy Whitcroft]

Juergen Ruehle wrote:
> From: =?iso-8859-1?q?J=FCrgen_R=FChle?= <j-r@online.de>
> 
> This tries to be more to the point while also including a pointer on how to
> unstage changes from the index.
> 
> Since this header is printed in two different code paths and the name of the
> reference commit is needed for the unstage part, provide a new printing
> function.
> 
> Signed-off-by: Jürgen Rühle <j-r@online.de>
> ---
>  wt-status.c |   18 ++++++++++++++----
>  1 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/wt-status.c b/wt-status.c
> index 34be91b..98ca135 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -53,6 +53,18 @@ void wt_status_prepare(struct wt_status *s)
>  	s->untracked = 0;
>  }
>  
> +static void wt_status_print_cached_header(const char *reference)
> +{
> +	const char *c = color(WT_STATUS_HEADER);
> +	color_printf_ln(c, "# Cached changes to be committed:");
> +	if (reference) {
> +		color_printf_ln(c, "#   (use \"git reset %s <file>...\" and \"git rm --cached <file>...\" to unstage)", reference);
> +	} else {
> +		color_printf_ln(c, "#   (use \"git rm --cached <file>...\" to unstage)");
> +	}
> +	color_printf_ln(c, "#");

We seem to be using 'Cached' and 'staged' here.  I thought we had
updated all the docs to call files in the index "Staged for commit".  It
feels like the header should be more like:

     # Currently staged changes, will be commited:

The rm --cached is somewhat unfortunate either way.

-apw

Re: [PATCH 2/4] Improve cached content header of status output

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

>> +static void wt_status_print_cached_header(const char *reference)
>> +{
>> +	const char *c = color(WT_STATUS_HEADER);
>> +	color_printf_ln(c, "# Cached changes to be committed:");
>> ...
>
> We seem to be using 'Cached' and 'staged' here.  I thought we had
> updated all the docs to call files in the index "Staged for commit".

	# Changes staged to be committed:

Somebody did not like the verb "stage"; perhaps we can say:

	# You have added changes to these files to be committed:
	...
        # There are yet to be added changes to these files:
	...

?

Re: [PATCH 2/4] Improve cached content header of status output

[Andy Whitcroft]

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>>> +static void wt_status_print_cached_header(const char *reference)
>>> +{
>>> +	const char *c = color(WT_STATUS_HEADER);
>>> +	color_printf_ln(c, "# Cached changes to be committed:");
>>> ...
>> We seem to be using 'Cached' and 'staged' here.  I thought we had
>> updated all the docs to call files in the index "Staged for commit".
> 
> 	# Changes staged to be committed:
> 
> Somebody did not like the verb "stage"; perhaps we can say:
> 
> 	# You have added changes to these files to be committed:
> 	...

# These files have changes and are marked for commit:

>         # There are yet to be added changes to these files:

# These files have changes but are not marked for commit:

> 	...

-apw

Re: [PATCH 2/4] Improve cached content header of status output

[Juergen Ruehle]

Andy Whitcroft writes:
 > Junio C Hamano wrote:
 > > Andy Whitcroft <apw@shadowen.org> writes:
 > > 
 > >>> +static void wt_status_print_cached_header(const char *reference)
 > >>> +{
 > >>> +	const char *c = color(WT_STATUS_HEADER);
 > >>> +	color_printf_ln(c, "# Cached changes to be committed:");
 > >>> ...
 > >> We seem to be using 'Cached' and 'staged' here.  I thought we had
 > >> updated all the docs to call files in the index "Staged for commit".
 > > 
 > > 	# Changes staged to be committed:
 > > 
 > > Somebody did not like the verb "stage"; perhaps we can say:
 > > 
 > > 	# You have added changes to these files to be committed:
 > > 	...
 > 
 > # These files have changes and are marked for commit:
 > 
 > >         # There are yet to be added changes to these files:
 > 
 > # These files have changes but are not marked for commit:

Does this better reflect that git tracks content and not files?

# Changes to these files will be committed:

# Changes to these files are not marked for commit:

BTW: how about also adding a hint how to review the changes in
question (i.e. diff --cached and diff; as an alternative to diff
--cached we could just advertise the --verbose switch to status and
commit).

Re: [PATCH 2/4] Improve cached content header of status output

[Junio C Hamano]

Juergen Ruehle <j.ruehle@bmiag.de> writes:

> Andy Whitcroft writes:
>  > Junio C Hamano wrote:
>  > > 
>  > > Somebody did not like the verb "stage"; perhaps we can say:
>  > > 
>  > > 	# You have added changes to these files to be committed:
>  > > 	...
>  > 
>  > # These files have changes and are marked for commit:
>  > 
>  > >         # There are yet to be added changes to these files:
>  > 
>  > # These files have changes but are not marked for commit:
>
> Does this better reflect that git tracks content and not files?
>
> # Changes to these files will be committed:
>
> # Changes to these files are not marked for commit:

One of the goals is to find a pair of messages that make sense
when the same file appears on both lists.

> BTW: how about also adding a hint how to review the changes in
> question (i.e. diff --cached and diff; as an alternative to diff
> --cached we could just advertise the --verbose switch to status and
> commit).

Sounds sane.

Re: [PATCH 2/4] Improve cached content header of status output

[Andy Whitcroft]

Junio C Hamano wrote:
> Juergen Ruehle <j.ruehle@bmiag.de> writes:
> 
>> Andy Whitcroft writes:
>>  > Junio C Hamano wrote:
>>  > > 
>>  > > Somebody did not like the verb "stage"; perhaps we can say:
>>  > > 
>>  > > 	# You have added changes to these files to be committed:
>>  > > 	...
>>  > 
>>  > # These files have changes and are marked for commit:
>>  > 
>>  > >         # There are yet to be added changes to these files:
>>  > 
>>  > # These files have changes but are not marked for commit:
>>
>> Does this better reflect that git tracks content and not files?
>>
>> # Changes to these files will be committed:
>>
>> # Changes to these files are not marked for commit:
> 
> One of the goals is to find a pair of messages that make sense
> when the same file appears on both lists.

Doh, double changes ... yes.

I am not sure it is possible to sanely textualise that subtlety in a
single line.  I wonder if its worth splitting this lot into three.
Basically those files on list one, those on list two and those on both.

Anyhow, lets see if we can textualise:

# Changes to these files will be committed:

# The latest changes to these files will not be committed:

The first here still implies its the latest changes.  I can not
trivially word round that.  Perhaps we could mention staging?

# Staged changes for these files will be commited:

# These files have unstaged changes which will not be committed:

>> BTW: how about also adding a hint how to review the changes in
>> question (i.e. diff --cached and diff; as an alternative to diff
>> --cached we could just advertise the --verbose switch to status and
>> commit).
> 
> Sounds sane.


-apw

Re: [PATCH 2/4] Improve cached content header of status output

[Juergen Ruehle]

Andy Whitcroft writes:
 > Junio C Hamano wrote:
 > > Juergen Ruehle <j.ruehle@bmiag.de> writes:
 > >> Does this better reflect that git tracks content and not files?
 > >>
 > >> # Changes to these files will be committed:
 > >>
 > >> # Changes to these files are not marked for commit:
 > > 
 > > One of the goals is to find a pair of messages that make sense
 > > when the same file appears on both lists.
 > 
 > Doh, double changes ... yes.
 > 
 > I am not sure it is possible to sanely textualise that subtlety in a
 > single line.  I wonder if its worth splitting this lot into three.
 > Basically those files on list one, those on list two and those on both.
 > 
 > Anyhow, lets see if we can textualise:
 > 
 > # Changes to these files will be committed:
 > 
 > # The latest changes to these files will not be committed:
 > 
 > The first here still implies its the latest changes.  I can not
 > trivially word round that.  Perhaps we could mention staging?
 > 
 > # Staged changes for these files will be commited:
 > 
 > # These files have unstaged changes which will not be committed:
 > 
 > >> BTW: how about also adding a hint how to review the changes in
 > >> question (i.e. diff --cached and diff; as an alternative to diff
 > >> --cached we could just advertise the --verbose switch to status and
 > >> commit).
 > > 
 > > Sounds sane.

Together with an appropriate second line it is looking good to me

# Staged changes for these files will be commited:
#   (use --verbose to review and "git reset HEAD <file>..." to unstage)

# These files have unstaged changes which will not be committed:
#   (Use "git diff" to review and "git add <file>..." to stage for commit)

Perhaps we do not even need to mention files?

# Staged changes to be commited:
#   (use --verbose to review and "git reset HEAD <file>..." to unstage)

# Unstaged changes to the working directory which will not be committed:
#   (Use "git diff" to review and "git add <file>..." to stage for commit)

[PATCH 3/4] Improve "nothing to commit" part of status output

[Juergen Ruehle]

From: =?iso-8859-1?q?J=FCrgen_R=FChle?= <j-r@online.de>

Previously git-status in a clean working directory would advice the user to use
git add. This isn't very helpful when there is nothing to add in the working
directory, therefore note a clean working directory while displaying the other
sections and print the appropriate message for each case.

Signed-off-by: Jürgen Rühle <j-r@online.de>
---
 wt-status.c |   20 +++++++++++++++-----
 wt-status.h |    1 +
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 98ca135..302f7ee 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -51,6 +51,8 @@ void wt_status_prepare(struct wt_status *s)
 	s->verbose = 0;
 	s->commitable = 0;
 	s->untracked = 0;
+
+	s->workdir_clean = 1;
 }
 
 static void wt_status_print_cached_header(const char *reference)
@@ -173,9 +175,12 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
                         struct diff_options *options,
                         void *data)
 {
+	struct wt_status *s = data;
 	int i;
-	if (q->nr)
+	if (q->nr) {
+		s->workdir_clean = 0;
 		wt_status_print_header("Changed but not added", use_add_msg);
+	}
 	for (i = 0; i < q->nr; i++)
 		wt_status_print_filepair(WT_STATUS_CHANGED, q->queue[i]);
 	if (q->nr)
@@ -225,7 +230,7 @@ static void wt_status_print_changed(struct wt_status *s)
 	run_diff_files(&rev, 0);
 }
 
-static void wt_status_print_untracked(const struct wt_status *s)
+static void wt_status_print_untracked(struct wt_status *s)
 {
 	struct dir_struct dir;
 	const char *x;
@@ -260,6 +265,7 @@ static void wt_status_print_untracked(const struct wt_status *s)
 				continue;
 		}
 		if (!shown_header) {
+			s->workdir_clean = 0;
 			wt_status_print_header("Untracked files", use_add_msg);
 			shown_header = 1;
 		}
@@ -302,9 +308,13 @@ void wt_status_print(struct wt_status *s)
 	if (s->verbose && !s->is_initial)
 		wt_status_print_verbose(s);
 	if (!s->commitable)
-		printf("%s (%s)\n",
-			s->amend ? "# No changes" : "nothing to commit",
-			use_add_msg);
+		if (s->amend)
+			printf("# No changes\n");
+		else if (s->workdir_clean)
+			printf(s->is_initial ? "nothing to commit\n"
+				: "nothing to commit (working directory matches HEAD)\n");
+		else
+			printf("no changes added to commit (use \"git add\" and/or \"git commit [-a|-i|-o]\")\n");
 }
 
 int git_status_config(const char *k, const char *v)
diff --git a/wt-status.h b/wt-status.h
index 0a5a5b7..892a86c 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -16,6 +16,7 @@ struct wt_status {
 	int verbose;
 	int amend;
 	int untracked;
+	int workdir_clean;
 };
 
 int git_status_config(const char *var, const char *value);
-- 
1.5.0.rc0.g31e2

[PATCH 4/4] Support --amend on initial commit in status output

[Juergen Ruehle]

From: =?iso-8859-1?q?J=FCrgen_R=FChle?= <j-r@online.de>

We check the existence of the parent commit to determine whether the status is
requested for an initial commit. Since the parent commit depends on the
presence of the --amend switch do initial commit detection after command line
arguments have been handled.

Signed-off-by: Jürgen Rühle <j-r@online.de>
---
 wt-status.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 302f7ee..166d0e3 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -41,8 +41,6 @@ void wt_status_prepare(struct wt_status *s)
 	unsigned char sha1[20];
 	const char *head;
 
-	s->is_initial = get_sha1("HEAD", sha1) ? 1 : 0;
-
 	head = resolve_ref("HEAD", sha1, 0, NULL);
 	s->branch = head ? xstrdup(head) : NULL;
 
@@ -287,6 +285,9 @@ static void wt_status_print_verbose(struct wt_status *s)
 
 void wt_status_print(struct wt_status *s)
 {
+	unsigned char sha1[20];
+	s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+
 	if (s->branch)
 		color_printf_ln(color(WT_STATUS_HEADER),
 			"# On branch %s", s->branch);
-- 
1.5.0.rc0.g31e2

Re: [PATCH/RFC] Assorted small changes to runstatus

[Junio C Hamano]

Thanks.  3 and 4 are obviously good, 1 is probably good although
I do not have much preference either way myself.  The part that
adds helpful messages of 2 is nice but I am not sure about the
wording "Cached changes".

Maybe people who were in the update-index and git-add discussion
have better input than I can give.

Re: [PATCH/RFC] Assorted small changes to runstatus

[Juergen Ruehle]

Junio C Hamano writes:
 > Thanks.  3 and 4 are obviously good, 1 is probably good although
 > I do not have much preference either way myself.  The part that
 > adds helpful messages of 2 is nice but I am not sure about the
 > wording "Cached changes".
 > 
 > Maybe people who were in the update-index and git-add discussion
 > have better input than I can give.

Yes, that was the intention. Thanks for looking at the proposal.

[PATCH] Remove unnecessary git-rm --cached reference from status output

[Juergen Ruehle]

From: =?utf-8?q?J=C3=BCrgen_R=C3=BChle?= <j-r@online.de>

Since git-reset has learned restoring the absence of paths git-rm --cached is
no longer necessary. Therefore remove it from the cached content header hint.

Also remove the unfortunate wording 'Cached' from the header itself.

Signed-off-by: Jürgen Rühle <j-r@online.de>
---
 !!This is based on next (or the jr/status series)!!

 This does not yet implement the other proposed changes to the header
 lines, pending further feedback.

 wt-status.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 8aac526..c48127d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -56,9 +56,9 @@ void wt_status_prepare(struct wt_status *s)
 static void wt_status_print_cached_header(const char *reference)
 {
 	const char *c = color(WT_STATUS_HEADER);
-	color_printf_ln(c, "# Cached changes to be committed:");
+	color_printf_ln(c, "# Changes to be committed:");
 	if (reference) {
-		color_printf_ln(c, "#   (use \"git reset %s <file>...\" and \"git rm --cached <file>...\" to unstage)", reference);
+		color_printf_ln(c, "#   (use \"git reset %s <file>...\" to unstage)", reference);
 	} else {
 		color_printf_ln(c, "#   (use \"git rm --cached <file>...\" to unstage)");
 	}
-- 
1.5.0.rc0.g525e