[PATCH 1/1] status: display "doing what" information in git status

[PATCH 1/1] status: display "doing what" information in git status

[Pierre Habouzit]

This provides the same information as the git bash prompt about the
current operation that is going on: rebase, merge, am, cherry-pick or
bisect.

This is very useful for "beginners" who don't have their shell prompt set
up for git.

The logic has been largely borrowed from
contrib/completion/git-completion.bash

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 wt-status.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 9f4e0ba..aad0f7f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -527,6 +527,79 @@ static void wt_status_print_unmerged(struct wt_status *s)
 
 }
 
+static int wt_has_file(const char *what)
+{
+	char path[PATH_MAX];
+
+	return access(git_snpath(path, sizeof(path), "%s", what), F_OK) == 0;
+}
+
+static void wt_status_print_doingwhat(struct wt_status *s)
+{
+	const char *status_header   = color(WT_STATUS_HEADER, s);
+	const char *status_nobranch = color(WT_STATUS_NOBRANCH, s);
+	struct strbuf sb = STRBUF_INIT;
+	char path[PATH_MAX];
+
+	if (wt_has_file("rebase-merge/interactive")) {
+		status_printf(s, status_header, "");
+
+		git_snpath(path, sizeof(path), "%s", "rebase-merge/head-name");
+		strbuf_read_file(&sb, path, 0);
+		strbuf_rtrim(&sb);
+
+		status_printf_more(s, status_nobranch,
+				   _("in the middle of a git rebase -i of %s"),
+				   prefixcmp(sb.buf, "refs/heads/") == 0 ?
+				   sb.buf + 11 : sb.buf);
+	} else if (wt_has_file("rebase-merge")) {
+		status_printf(s, status_header, "");
+
+		git_snpath(path, sizeof(path), "%s", "rebase-merge/head-name");
+		strbuf_read_file(&sb, path, 0);
+		strbuf_rtrim(&sb);
+
+		status_printf_more(s, status_nobranch,
+				   _("in the middle of a git rebase -m on %s"),
+				   prefixcmp(sb.buf, "refs/heads/") == 0 ?
+				   sb.buf + 11 : sb.buf);
+	} else {
+		if (wt_has_file("rebase-apply")) {
+			status_printf(s, status_header, "");
+
+			if (wt_has_file("rebase-apply/rebasing")) {
+				status_printf_more(s, status_nobranch,
+						   _("in the middle of a git rebase"));
+			} else if (wt_has_file("rebase-apply/applying")) {
+				status_printf_more(s, status_nobranch,
+						   _("in the middle of a git am"));
+			} else {
+				status_printf_more(s, status_nobranch,
+						   _("in the middle of a git rebase or am"));
+			}
+		} else if (wt_has_file("MERGE_HEAD")) {
+			status_printf(s, status_header, "");
+
+			status_printf_more(s, status_nobranch,
+					   _("in the middle of a git merge"));
+		} else if (wt_has_file("CHERRY_PICK_HEAD")) {
+			status_printf(s, status_header, "");
+
+			status_printf_more(s, status_nobranch,
+					   _("in the middle of a git cherry-pick"));
+		} else if (wt_has_file("BISECT_LOG")) {
+			status_printf(s, status_header, "");
+
+			status_printf_more(s, status_nobranch,
+					   _("in the middle of a git bisect"));
+		} else {
+			return;
+		}
+	}
+	status_printf_more(s, status_header, "\n");
+	strbuf_release(&sb);
+}
+
 static void wt_status_print_updated(struct wt_status *s)
 {
 	int shown_header = 0;
@@ -732,6 +805,7 @@ void wt_status_print(struct wt_status *s)
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
 	}
 
+	wt_status_print_doingwhat(s);
 	wt_status_print_updated(s);
 	wt_status_print_unmerged(s);
 	wt_status_print_changed(s);
-- 
1.7.5.1.289.g76e37.dirty

Re: [PATCH 1/1] status: display "doing what" information in git status

[Sverre Rabbelier]

Heya,

On Thu, May 5, 2011 at 23:48, Pierre Habouzit <madcoder@debian.org> wrote:
> This provides the same information as the git bash prompt about the
> current operation that is going on: rebase, merge, am, cherry-pick or
> bisect.

Can you show how this will look like?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH 1/1] status: display "doing what" information in git status

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 958 bytes --]

On Fri, May 06, 2011 at 01:06:45AM +0200, Sverre Rabbelier wrote:
> Heya,
> 
> On Thu, May 5, 2011 at 23:48, Pierre Habouzit <madcoder@debian.org> wrote:
> > This provides the same information as the git bash prompt about the
> > current operation that is going on: rebase, merge, am, cherry-pick or
> > bisect.
> 
> Can you show how this will look like?

Sure, it adds a line on the top with the same color as "not on any
branch" iff there is an ongoing operation.

Of course in this setup it makes no sense since my shell shows it
already, but I'm frustrated when I use git on a remote machine where I
don't have zsh installed or configured, and at work many people would
like to know where they left stuff before they grabbed coffee and talked
for 1h instead of taking 5 minutes ;)
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: scr.png --]
[-- Type: image/png, Size: 9428 bytes --]

Re: [PATCH 1/1] status: display "doing what" information in git status

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> @@ -732,6 +805,7 @@ void wt_status_print(struct wt_status *s)
>  		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
>  	}
>  
> +	wt_status_print_doingwhat(s);
>  	wt_status_print_updated(s);
>  	wt_status_print_unmerged(s);
>  	wt_status_print_changed(s);

I am very surprised that a new call to this function is added here.

As the "You are in middle of" information is useful mostly when you are on
detached head, I would have expected that the call would be inside the
if/elif chain near the top of wt_status_print() where we say "On branch"
vs "Not currently on...", to replace that information that comes from that
on_what variable.

Re: [PATCH 1/1] status: display "doing what" information in git status

[Pierre Habouzit]

On Thu, May 05, 2011 at 04:37:00PM -0700, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > @@ -732,6 +805,7 @@ void wt_status_print(struct wt_status *s)
> >  		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
> >  	}
> >  
> > +	wt_status_print_doingwhat(s);
> >  	wt_status_print_updated(s);
> >  	wt_status_print_unmerged(s);
> >  	wt_status_print_changed(s);
> 
> I am very surprised that a new call to this function is added here.
> 
> As the "You are in middle of" information is useful mostly when you are on
> detached head, I would have expected that the call would be inside the
> if/elif chain near the top of wt_status_print() where we say "On branch"
> vs "Not currently on...", to replace that information that comes from that
> on_what variable.

It's also useful when you're in the middle of a rebase or during a merge
conflict who aren't on a detached head.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Re: [PATCH 1/1] status: display "doing what" information in git status

[Pierre Habouzit]

On Fri, May 06, 2011 at 01:39:25AM +0200, Pierre Habouzit wrote:
> On Thu, May 05, 2011 at 04:37:00PM -0700, Junio C Hamano wrote:
> > Pierre Habouzit <madcoder@debian.org> writes:
> > 
> > > @@ -732,6 +805,7 @@ void wt_status_print(struct wt_status *s)
> > >  		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
> > >  	}
> > >  
> > > +	wt_status_print_doingwhat(s);
> > >  	wt_status_print_updated(s);
> > >  	wt_status_print_unmerged(s);
> > >  	wt_status_print_changed(s);
> > 
> > I am very surprised that a new call to this function is added here.
> > 
> > As the "You are in middle of" information is useful mostly when you are on
> > detached head, I would have expected that the call would be inside the
> > if/elif chain near the top of wt_status_print() where we say "On branch"
> > vs "Not currently on...", to replace that information that comes from that
> > on_what variable.
> 
> It's also useful when you're in the middle of a rebase or during a merge
> conflict who aren't on a detached head.

s/rebase/bisect/ sorry it's late.

Anyways it prints nothing if the repository "looks normal"
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Re: [PATCH 1/1] status: display "doing what" information in git status

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

>> As the "You are in middle of" information is useful mostly when you are on
>> detached head, I would have expected that the call would be inside the
>> if/elif chain near the top of wt_status_print() where we say "On branch"
>> vs "Not currently on...", to replace that information that comes from that
>> on_what variable.
>
> It's also useful when you're in the middle of a rebase or during a merge
> conflict who aren't on a detached head.

What I would expect in such a case would be like:

	# On branch master (resolving merge with ph/status)

instead of wasting an extra line like:

	# Resolving a merge conflict
        # On branch master

That is what I meant "to replace".

Re: [PATCH 1/1] status: display "doing what" information in git status

[Pierre Habouzit]

On Thu, May 05, 2011 at 04:49:50PM -0700, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> >> As the "You are in middle of" information is useful mostly when you are on
> >> detached head, I would have expected that the call would be inside the
> >> if/elif chain near the top of wt_status_print() where we say "On branch"
> >> vs "Not currently on...", to replace that information that comes from that
> >> on_what variable.
> >
> > It's also useful when you're in the middle of a rebase or during a merge
> > conflict who aren't on a detached head.
> 
> What I would expect in such a case would be like:
> 
> 	# On branch master (resolving merge with ph/status)
> 
> instead of wasting an extra line like:
> 
> 	# Resolving a merge conflict
>         # On branch master
> 
> That is what I meant "to replace".

okay it's really late, sorry, I read too fast, I'll work on something
like that, doesn't looks too complicated ;)

I'll post an updated patch on tomorrow.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[PATCH v2] status: display "doing what" information in git status

[Pierre Habouzit]

This provides the same information as the git bash prompt about the
current operation that is going on: rebase, merge, am, cherry-pick or
bisect.

This is very useful for "beginners" who don't have their shell prompt set
up for git.

The logic has been largely borrowed from
contrib/completion/git-completion.bash

when hints are enabled, it also gives hints on how to abort or resolve
that pending situation. For example, with this patch and hints activated,
being in the middle of a `git rebase -i` looks like:

	$ git status
	# in the middle of a git rebase -i of master (detached head)
	#   (use "git rebase --abort" to abort current rebase or proceed)
	# Changes not staged for commit:
	#   (use "git add <file>..." to update what will be committed)
	#   (use "git checkout -- <file>..." to discard changes in working directory)
	#
	#       modified:   lib-common (new commits)
	#
	# Submodules changed but not updated:
	#
	# * lib-common 0b07a6b...11cdf27 (1):
	#   > Add t_sb_init().
	#
	# Untracked files:
	#   (use "git add <file>..." to include in what will be committed)
	#
	#       qdb/a/
	#       qkv/A/
	no changes added to commit (use "git add" and/or "git commit -a")

If we have an ongoing operation then:
- if we are on a branch it displays:
  # On branch $branch ($what_is_ongoing)
  #   ($ongoing_hint)
- if we are on a detached head it displays:
  # $what_is_ongoing (detached head)
  #   ($ongoing_hint)

If we have no ongoing operation the git status does as before:
- if we are on a branch it displays:
  # On branch $branch
- if we are on a detached head it displays:
  # Not currently on any branch.

Since the ongoing operation is usually something to be done with before
continuing with further git operations, the hint and ongoing operations
are displayed with the "WT_STATUS_NOBRANCH" color to be easy to spot.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 wt-status.c |  138 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 123 insertions(+), 15 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 9f4e0ba..91f6c7c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -527,6 +527,128 @@ static void wt_status_print_unmerged(struct wt_status *s)
 
 }
 
+static int wt_has_file(const char *what)
+{
+	char path[PATH_MAX];
+
+	return access(git_snpath(path, sizeof(path), "%s", what), F_OK) == 0;
+}
+
+static void wt_status_print_doingwhat(struct wt_status *s)
+{
+	const char *status_nobranch = color(WT_STATUS_NOBRANCH, s);
+	const char *branch_name = s->branch;
+	const char *advice = NULL;
+
+	const char * const rebase_advice =
+		_("use \"git rebase --abort\" to abort current rebase or proceed");
+	const char * const am_advice =
+		_("use \"git am --abort\" to abort current mailbox apply or proceed");
+	const char * const merge_advice =
+		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");
+
+	struct strbuf sb = STRBUF_INIT;
+	char path[PATH_MAX];
+
+	if (!prefixcmp(branch_name, "refs/heads/"))
+		branch_name += 11;
+	else if (!strcmp(branch_name, "HEAD")) {
+		branch_name = NULL;
+	}
+
+	status_printf(s, color(WT_STATUS_HEADER, s), "");
+	if (branch_name) {
+		const char *status_onbranch = color(WT_STATUS_ONBRANCH, s);
+
+		status_printf_more(s, status_onbranch, _("On branch "));
+		status_printf_more(s, status_onbranch, "%s", branch_name);
+	}
+	if (wt_has_file("rebase-merge")) {
+		git_snpath(path, sizeof(path), "%s", "rebase-merge/head-name");
+		strbuf_read_file(&sb, path, 0);
+		strbuf_rtrim(&sb);
+
+		if (branch_name)
+			status_printf_more(s, status_nobranch, " (");
+		status_printf_more(s, status_nobranch,
+				   _("in the middle of a git rebase -%c of %s"),
+				   wt_has_file("rebase-merge/interactive") ? 'i' : 'm',
+				   prefixcmp(sb.buf, "refs/heads/") == 0 ?
+				   sb.buf + 11 : sb.buf);
+		advice = rebase_advice;
+	} else if (wt_has_file("rebase-apply")) {
+		if (branch_name)
+			status_printf_more(s, status_nobranch, " (");
+		if (wt_has_file("rebase-apply/rebasing")) {
+			status_printf_more(s, status_nobranch,
+					   _("in the middle of a git rebase"));
+			advice = rebase_advice;
+		} else if (wt_has_file("rebase-apply/applying")) {
+			status_printf_more(s, status_nobranch,
+					   _("in the middle of a git am"));
+			advice = am_advice;
+		} else {
+			status_printf_more(s, status_nobranch,
+					   _("in the middle of a git rebase/am"));
+			advice = am_advice;
+		}
+	} else if (wt_has_file("MERGE_HEAD")) {
+		git_snpath(path, sizeof(path), "%s", "MERGE_HEAD");
+		strbuf_read_file(&sb, path, 0);
+		strbuf_rtrim(&sb);
+
+		if (branch_name)
+			status_printf_more(s, status_nobranch, " (");
+		status_printf_more(s, status_nobranch,
+				   _("resolving merge with %s"),
+				   prefixcmp(sb.buf, "refs/heads/") == 0 ?
+				   sb.buf + 11 : sb.buf);
+		advice = merge_advice;
+	} else if (wt_has_file("CHERRY_PICK_HEAD")) {
+		unsigned char sha1buf[20];
+
+		git_snpath(path, sizeof(path), "%s", "CHERRY_PICK_HEAD");
+		strbuf_read_file(&sb, path, 0);
+		strbuf_rtrim(&sb);
+
+		if (branch_name)
+			status_printf_more(s, status_nobranch, " (");
+		if (sb.len == 40 && get_sha1_hex(sb.buf, sha1buf) == 0) {
+			status_printf_more(s, status_nobranch,
+					   _("resolving cherry-pick of %s"),
+					   find_unique_abbrev(sha1buf, DEFAULT_ABBREV));
+		} else {
+			status_printf_more(s, status_nobranch,
+					   _("resolving cherry-pick of %s"),
+					   sb.buf);
+		}
+		advice = merge_advice;
+	} else if (wt_has_file("BISECT_LOG")) {
+		if (branch_name)
+			status_printf_more(s, status_nobranch, " (");
+		status_printf_more(s, status_nobranch,
+					   _("in the middle of a git bisect"));
+	} else {
+		if (!branch_name) {
+			status_printf_more(s, status_nobranch,
+					   _("Not currently on any branch."));
+		}
+		status_printf_more(s, "", "\n");
+		return;
+	}
+
+	if (branch_name) {
+		status_printf_more(s, status_nobranch, ")\n");
+	} else {
+		status_printf_more(s, status_nobranch, _(" (detached head)\n"));
+	}
+	if (advice && advice_status_hints) {
+		status_printf(s, color(WT_STATUS_HEADER, s), "");
+		status_printf_more(s, status_nobranch, "  (%s)\n", advice);
+	}
+	strbuf_release(&sb);
+}
+
 static void wt_status_print_updated(struct wt_status *s)
 {
 	int shown_header = 0;
@@ -706,22 +828,8 @@ static void wt_status_print_tracking(struct wt_status *s)
 
 void wt_status_print(struct wt_status *s)
 {
-	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
-	const char *branch_status_color = color(WT_STATUS_HEADER, s);
-
 	if (s->branch) {
-		const char *on_what = _("On branch ");
-		const char *branch_name = s->branch;
-		if (!prefixcmp(branch_name, "refs/heads/"))
-			branch_name += 11;
-		else if (!strcmp(branch_name, "HEAD")) {
-			branch_name = "";
-			branch_status_color = color(WT_STATUS_NOBRANCH, s);
-			on_what = _("Not currently on any branch.");
-		}
-		status_printf(s, color(WT_STATUS_HEADER, s), "");
-		status_printf_more(s, branch_status_color, "%s", on_what);
-		status_printf_more(s, branch_color, "%s\n", branch_name);
+		wt_status_print_doingwhat(s);
 		if (!s->is_initial)
 			wt_status_print_tracking(s);
 	}
-- 
1.7.5.1.290.gfe86e2.dirty

Re: [PATCH 1/1] status: display "doing what" information in git status

[Michael J Gruber]

Pierre Habouzit venit, vidit, dixit 06.05.2011 01:26:
> On Fri, May 06, 2011 at 01:06:45AM +0200, Sverre Rabbelier wrote:
>> Heya,
>>
>> On Thu, May 5, 2011 at 23:48, Pierre Habouzit <madcoder@debian.org> wrote:
>>> This provides the same information as the git bash prompt about the
>>> current operation that is going on: rebase, merge, am, cherry-pick or
>>> bisect.
>>
>> Can you show how this will look like?
> 
> Sure, it adds a line on the top with the same color as "not on any
> branch" iff there is an ongoing operation.
> 
> Of course in this setup it makes no sense since my shell shows it
> already, but I'm frustrated when I use git on a remote machine where I
> don't have zsh installed or configured, and at work many people would
> like to know where they left stuff before they grabbed coffee and talked
> for 1h instead of taking 5 minutes ;)

I think this is useful and nice in the compactified version suggested by
Junio. Be prepared for someone requesting it with "status -s -b" :)

What became of the colouring of the git-prompt, btw? I see you're using
some, and I remember a stalled effort to have this in our shipped
completion. Do have something shareable in that respect?

Michael

Re: [PATCH 1/1] status: display "doing what" information in git status

[Pierre Habouzit]

On Fri, May 06, 2011 at 09:48:52AM +0200, Michael J Gruber wrote:
> Pierre Habouzit venit, vidit, dixit 06.05.2011 01:26:
> > On Fri, May 06, 2011 at 01:06:45AM +0200, Sverre Rabbelier wrote:
> >> Heya,
> >>
> >> On Thu, May 5, 2011 at 23:48, Pierre Habouzit <madcoder@debian.org> wrote:
> >>> This provides the same information as the git bash prompt about the
> >>> current operation that is going on: rebase, merge, am, cherry-pick or
> >>> bisect.
> >>
> >> Can you show how this will look like?
> > 
> > Sure, it adds a line on the top with the same color as "not on any
> > branch" iff there is an ongoing operation.
> > 
> > Of course in this setup it makes no sense since my shell shows it
> > already, but I'm frustrated when I use git on a remote machine where I
> > don't have zsh installed or configured, and at work many people would
> > like to know where they left stuff before they grabbed coffee and talked
> > for 1h instead of taking 5 minutes ;)
> 
> I think this is useful and nice in the compactified version suggested by
> Junio. Be prepared for someone requesting it with "status -s -b" :)

Well, I've written the logic, it's easy to use :P

> What became of the colouring of the git-prompt, btw? I see you're using
> some, and I remember a stalled effort to have this in our shipped
> completion. Do have something shareable in that respect?

My git prompt predates the bash one, it's here
http://madism.org/~madcoder/dotfiles/config/zsh/60_prompt you're
free to do what you want with that.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Re: [PATCH v2] status: display "doing what" information in git status

[Jakub Narebski]

Pierre Habouzit <madcoder@debian.org> writes:

> This provides the same information as the git bash prompt about the
> current operation that is going on: rebase, merge, am, cherry-pick or
> bisect.
> 
> This is very useful for "beginners" who don't have their shell prompt set
> up for git.
> 
> The logic has been largely borrowed from
> contrib/completion/git-completion.bash

Very nice idea.
 
> when hints are enabled, it also gives hints on how to abort or resolve

s/^when/When/

> If we have an ongoing operation then:
> - if we are on a branch it displays:
>   # On branch $branch ($what_is_ongoing)
>   #   ($ongoing_hint)
> - if we are on a detached head it displays:
>   # $what_is_ongoing (detached head)
>   #   ($ongoing_hint)
> 
> If we have no ongoing operation the git status does as before:
> - if we are on a branch it displays:
>   # On branch $branch
> - if we are on a detached head it displays:
>   # Not currently on any branch.

Very nice to have such example in commit message.
 
> Since the ongoing operation is usually something to be done with before
> continuing with further git operations, the hint and ongoing operations
> are displayed with the "WT_STATUS_NOBRANCH" color to be easy to spot.
> 
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>  wt-status.c |  138 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 123 insertions(+), 15 deletions(-)

Could you please add some *tests* for this new feature?  

Don't forget to mark it with C_LOCALE_OUTPUT or use test_i18ncmp /
/ test_i18ngrep in tests.
 
> +static void wt_status_print_doingwhat(struct wt_status *s)
> +{
> +	const char *status_nobranch = color(WT_STATUS_NOBRANCH, s);
> +	const char *branch_name = s->branch;
> +	const char *advice = NULL;
> +
> +	const char * const rebase_advice =
> +		_("use \"git rebase --abort\" to abort current rebase or proceed");
> +	const char * const am_advice =
> +		_("use \"git am --abort\" to abort current mailbox apply or proceed");
> +	const char * const merge_advice =
> +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");

Thanks for marking it up for i18n... though I am not sure if we should
use _() or N_() here...

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH v2] status: display "doing what" information in git status

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> 	$ git status
> 	# in the middle of a git rebase -i of master (detached head)
> ...
> 	#       qkv/A/
> 	no changes added to commit (use "git add" and/or "git commit -a")
>
> If we have an ongoing operation then:
> - if we are on a branch it displays:
>   # On branch $branch ($what_is_ongoing)
>   #   ($ongoing_hint)
> - if we are on a detached head it displays:
>   # $what_is_ongoing (detached head)
>   #   ($ongoing_hint)

I'll reindent the above to align these example output the earlier output sample
when I apply them.

> +	const char * const rebase_advice =
> +		_("use \"git rebase --abort\" to abort current rebase or proceed");
> +	const char * const am_advice =
> +		_("use \"git am --abort\" to abort current mailbox apply or proceed");

If the reader does not even know that the "--abort" option is used to
abort, " or proceed" needs to be followed by "by doing $this", if it wants
to have any practical value.  I would suggest dropping it.

> +	const char * const merge_advice =
> +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");

This codepath being in wt-status.c, I wonder if any of these advices could
trigger to appear in the log message editor when the user tries to run
"git commit".  If so, I suspect any of the above is too late to help the
user, no?

Also, should we make these conditional upon advice.status or something?

Re: [PATCH v2] status: display "doing what" information in git status

[Pierre Habouzit]

On Fri, May 06, 2011 at 10:29:21AM -0700, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > 	$ git status
> > 	# in the middle of a git rebase -i of master (detached head)
> > ...
> > 	#       qkv/A/
> > 	no changes added to commit (use "git add" and/or "git commit -a")
> >
> > If we have an ongoing operation then:
> > - if we are on a branch it displays:
> >   # On branch $branch ($what_is_ongoing)
> >   #   ($ongoing_hint)
> > - if we are on a detached head it displays:
> >   # $what_is_ongoing (detached head)
> >   #   ($ongoing_hint)
> 
> I'll reindent the above to align these example output the earlier output sample
> when I apply them.

Sure.

> > +	const char * const rebase_advice =
> > +		_("use \"git rebase --abort\" to abort current rebase or proceed");
> > +	const char * const am_advice =
> > +		_("use \"git am --abort\" to abort current mailbox apply or proceed");
> 
> If the reader does not even know that the "--abort" option is used to
> abort, " or proceed" needs to be followed by "by doing $this", if it wants
> to have any practical value.  I would suggest dropping it.
> 
> > +	const char * const merge_advice =
> > +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");
> This codepath being in wt-status.c, I wonder if any of these advices could
> trigger to appear in the log message editor when the user tries to run
> "git commit".  If so, I suspect any of the above is too late to help the
> user, no?

Well I'm not very happy with the advices, feel free to reword them in a better
way.

> Also, should we make these conditional upon advice.status or something?
they are
                          vvvvvvvvvvvvvvvvvvv
    +       if (advice && advice_status_hints) {
    +               status_printf(s, color(WT_STATUS_HEADER, s), "");
    +               status_printf_more(s, status_nobranch, "  (%s)\n", advice);
    +       }


-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Re: [PATCH v2] status: display "doing what" information in git status

[Pierre Habouzit]

On Fri, May 06, 2011 at 03:13:55AM -0700, Jakub Narebski wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> > If we have an ongoing operation then:
> > - if we are on a branch it displays:
> >   # On branch $branch ($what_is_ongoing)
> >   #   ($ongoing_hint)
> > - if we are on a detached head it displays:
> >   # $what_is_ongoing (detached head)
> >   #   ($ongoing_hint)
> >
> > If we have no ongoing operation the git status does as before:
> > - if we are on a branch it displays:
> >   # On branch $branch
> > - if we are on a detached head it displays:
> >   # Not currently on any branch.
>
> Very nice to have such example in commit message.
>
> > Since the ongoing operation is usually something to be done with before
> > continuing with further git operations, the hint and ongoing operations
> > are displayed with the "WT_STATUS_NOBRANCH" color to be easy to spot.
> >
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> >  wt-status.c |  138 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  1 files changed, 123 insertions(+), 15 deletions(-)
> 
> Could you please add some *tests* for this new feature?  
> 
> Don't forget to mark it with C_LOCALE_OUTPUT or use test_i18ncmp /
> / test_i18ngrep in tests.

Frankly I sadly don't have the time to write them, which is a very bad
excuse, this is a feature I wrote for $work and that I use and I thought
it would be nice to share.

Generating all the possible cases is just very long, and the test
framework changed too much since the last time I used it a few years ago
and I abandoned after 10minutes not being able to make it work properly.

> > +static void wt_status_print_doingwhat(struct wt_status *s)
> > +{
> > +	const char *status_nobranch = color(WT_STATUS_NOBRANCH, s);
> > +	const char *branch_name = s->branch;
> > +	const char *advice = NULL;
> > +
> > +	const char * const rebase_advice =
> > +		_("use \"git rebase --abort\" to abort current rebase or proceed");
> > +	const char * const am_advice =
> > +		_("use \"git am --abort\" to abort current mailbox apply or proceed");
> > +	const char * const merge_advice =
> > +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");
> 
> Thanks for marking it up for i18n... though I am not sure if we should
> use _() or N_() here...

It's _() you want to translate those. They are used through a
printf("  (%s)", ...) later that doesn't need i18n.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Re: [PATCH v2] status: display "doing what" information in git status

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

>> > +	const char * const rebase_advice =
>> > +		_("use \"git rebase --abort\" to abort current rebase or proceed");
>> > +	const char * const am_advice =
>> > +		_("use \"git am --abort\" to abort current mailbox apply or proceed");
>> 
>> If the reader does not even know that the "--abort" option is used to
>> abort, " or proceed" needs to be followed by "by doing $this", if it wants
>> to have any practical value.  I would suggest dropping it.
>> 
>> > +	const char * const merge_advice =
>> > +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");
>> This codepath being in wt-status.c, I wonder if any of these advices could
>> trigger to appear in the log message editor when the user tries to run
>> "git commit".  If so, I suspect any of the above is too late to help the
>> user, no?
>
> Well I'm not very happy with the advices, feel free to reword them in a better
> way.

I already suggested dropping it ;-)  Rewording to emptiness would be the
simplest and cleanest.

>> Also, should we make these conditional upon advice.status or something?
> they are
>                           vvvvvvvvvvvvvvvvvvv
>     +       if (advice && advice_status_hints) {
>     +               status_printf(s, color(WT_STATUS_HEADER, s), "");
>     +               status_printf_more(s, status_nobranch, "  (%s)\n", advice);
>     +       }

By the way, this change also breaks t7060, at least.

Re: [PATCH v2] status: display "doing what" information in git status

[Matthieu Moy]

Pierre Habouzit <madcoder@debian.org> writes:

> +	const char * const merge_advice =
> +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");

I think that should be "git reset --merge" (which preserves files not
touched by the merge, and makes sure "git merge; git reset" doesn't
permanently loose data).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2] status: display "doing what" information in git status

[Pierre Habouzit]

On Fri, May 06, 2011 at 08:40:04PM +0200, Matthieu Moy wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > +	const char * const merge_advice =
> > +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");
> 
> I think that should be "git reset --merge" (which preserves files not
> touched by the merge, and makes sure "git merge; git reset" doesn't
> permanently loose data).

Wow, I didn't know about that one, so /maybe/ the hint isn't that
useless after all :)
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Re: [PATCH v2] status: display "doing what" information in git status

[Matthieu Moy]

Pierre Habouzit <madcoder@debian.org> writes:

> On Fri, May 06, 2011 at 08:40:04PM +0200, Matthieu Moy wrote:
>> Pierre Habouzit <madcoder@debian.org> writes:
>> 
>> > +	const char * const merge_advice =
>> > +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");
>> 
>> I think that should be "git reset --merge" (which preserves files not
>> touched by the merge, and makes sure "git merge; git reset" doesn't
>> permanently loose data).
>
> Wow, I didn't know about that one, so /maybe/ the hint isn't that
> useless after all :)

Actually, there's very few reason to run "git reset --hard" these days,
and we should really consider it as a very dangerous command that should
not be advertized to beginners. There's almost always a safer
alternative:

* You want to cancel a merge => git reset --merge
  (doesn't touch files not impacted by merges)

* You want to change the commit your HEAD points to => git reset --keep
  (in case you have local changes, they won't be overridden)

* You want to discard local changes => git stash
  (you may not think these changes are important, but you may be wrong)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/