[PATCH 0/5] nd/branch-show-rebase-bisect-state updates

[PATCH 0/5] nd/branch-show-rebase-bisect-state updates

[Nguyễn Thái Ngọc Duy]

This round addresses the code sharing issue in the previous round and
shows a bit more info in both git-status and git-branch. Now when you
do "git checkout v1.8.0" or "git checkout origin/master", you should
see that ref in status/branch output, in addition to detached HEAD
status.

Nguyễn Thái Ngọc Duy (5):
  checkout: record full target ref in reflog
  wt-status: split wt_status_state parsing function out
  wt-status: move wt_status_get_state() out to wt_status_print()
  status: show the ref that is checked out, even if it's detached
  branch: show more information when HEAD is detached

 builtin/branch.c            |  25 +++++++-
 builtin/checkout.c          |  19 +++++-
 t/t6030-bisect-porcelain.sh |   2 +-
 wt-status.c                 | 139 ++++++++++++++++++++++++++++++++++----------
 wt-status.h                 |   6 +-
 5 files changed, 152 insertions(+), 39 deletions(-)

-- 
1.8.1.2.536.gf441e6d

[PATCH 1/5] checkout: record full target ref in reflog

[Nguyễn Thái Ngọc Duy]

This simplifies parsing later on because the parser does not need to
do dwim on the target (and later dwim may be ambiguous if new refs are
added).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/checkout.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..b11bd31 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -398,6 +398,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
 
 struct branch_info {
 	const char *name; /* The short name used */
+	const char *full_ref; /* The full name of a real ref */
 	const char *path; /* The full name of a real branch */
 	struct commit *commit; /* The named commit */
 };
@@ -589,7 +590,8 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 	if (!old_desc && old->commit)
 		old_desc = sha1_to_hex(old->commit->object.sha1);
 	strbuf_addf(&msg, "checkout: moving from %s to %s",
-		    old_desc ? old_desc : "(invalid)", new->name);
+		    old_desc ? old_desc : "(invalid)",
+		    new->full_ref ? new->full_ref : new->name);
 
 	if (!strcmp(new->name, "HEAD") && !new->path && !opts->force_detach) {
 		/* Nothing to do. */
@@ -907,10 +909,21 @@ static int parse_branchname_arg(int argc, const char **argv,
 	setup_branch_path(new);
 
 	if (!check_refname_format(new->path, 0) &&
-	    !read_ref(new->path, branch_rev))
+	    !read_ref(new->path, branch_rev)) {
 		hashcpy(rev, branch_rev);
-	else
+		new->full_ref = xstrdup(new->path);
+	} else {
+		char *real_ref = NULL;
+		unsigned char sha1[20];
 		new->path = NULL; /* not an existing branch */
+		if (dwim_ref(new->name, strlen(new->name), sha1,
+			     &real_ref) == 1 &&
+		    !hashcmp(sha1, rev)) {
+			new->full_ref = real_ref;
+			real_ref = NULL;
+		}
+		free(real_ref);
+	}
 
 	new->commit = lookup_commit_reference_gently(rev, 1);
 	if (!new->commit) {
-- 
1.8.1.2.536.gf441e6d

[PATCH 2/5] wt-status: split wt_status_state parsing function out

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 52 +++++++++++++++++++++++++++++++---------------------
 wt-status.h |  5 +++--
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index ef405d0..183aafe 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -970,7 +970,7 @@ static void show_bisect_in_progress(struct wt_status *s,
  * Extract branch information from rebase/bisect
  */
 static void read_and_strip_branch(struct strbuf *sb,
-				  const char **branch,
+				  char **branch,
 				  const char *path)
 {
 	unsigned char sha1[20];
@@ -994,52 +994,62 @@ static void read_and_strip_branch(struct strbuf *sb,
 		strbuf_addstr(sb, abbrev);
 		*branch = sb->buf;
 	} else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
-		;
+		*branch = NULL;
 	else			/* bisect */
 		*branch = sb->buf;
+	if (*branch)
+		*branch = xstrdup(*branch);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+void wt_status_get_state(struct wt_status_state *state)
 {
-	const char *state_color = color(WT_STATUS_HEADER, s);
 	struct strbuf branch = STRBUF_INIT;
 	struct strbuf onto = STRBUF_INIT;
-	struct wt_status_state state;
 	struct stat st;
 
-	memset(&state, 0, sizeof(state));
+	memset(state, 0, sizeof(*state));
 
 	if (!stat(git_path("MERGE_HEAD"), &st)) {
-		state.merge_in_progress = 1;
+		state->merge_in_progress = 1;
 	} else if (!stat(git_path("rebase-apply"), &st)) {
 		if (!stat(git_path("rebase-apply/applying"), &st)) {
-			state.am_in_progress = 1;
+			state->am_in_progress = 1;
 			if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
-				state.am_empty_patch = 1;
+				state->am_empty_patch = 1;
 		} else {
-			state.rebase_in_progress = 1;
-			read_and_strip_branch(&branch, &state.branch,
+			state->rebase_in_progress = 1;
+			read_and_strip_branch(&branch, &state->branch,
 					      "rebase-apply/head-name");
-			read_and_strip_branch(&onto, &state.onto,
+			read_and_strip_branch(&onto, &state->onto,
 					      "rebase-apply/onto");
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
-			state.rebase_interactive_in_progress = 1;
+			state->rebase_interactive_in_progress = 1;
 		else
-			state.rebase_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
+			state->rebase_in_progress = 1;
+		read_and_strip_branch(&branch, &state->branch,
 				      "rebase-merge/head-name");
-		read_and_strip_branch(&onto, &state.onto,
+		read_and_strip_branch(&onto, &state->onto,
 				      "rebase-merge/onto");
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
-		state.cherry_pick_in_progress = 1;
+		state->cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st)) {
-		state.bisect_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
+		state->bisect_in_progress = 1;
+		read_and_strip_branch(&branch, &state->branch,
 				      "BISECT_START");
 	}
+	strbuf_release(&branch);
+	strbuf_release(&onto);
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+	const char *state_color = color(WT_STATUS_HEADER, s);
+	struct wt_status_state state;
+
+	wt_status_get_state(&state);
 
 	if (state.merge_in_progress)
 		show_merge_in_progress(s, &state, state_color);
@@ -1051,8 +1061,8 @@ static void wt_status_print_state(struct wt_status *s)
 		show_cherry_pick_in_progress(s, &state, state_color);
 	if (state.bisect_in_progress)
 		show_bisect_in_progress(s, &state, state_color);
-	strbuf_release(&branch);
-	strbuf_release(&onto);
+	free(state.branch);
+	free(state.onto);
 }
 
 void wt_status_print(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 81e1dcf..5ddcbf6 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,13 +79,14 @@ struct wt_status_state {
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
-	const char *branch;
-	const char *onto;
+	char *branch;
+	char *onto;
 };
 
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
+void wt_status_get_state(struct wt_status_state *state);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

[PATCH 3/5] wt-status: move wt_status_get_state() out to wt_status_print()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 183aafe..6a3566b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1044,31 +1044,29 @@ void wt_status_get_state(struct wt_status_state *state)
 	strbuf_release(&onto);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_print_state(struct wt_status *s,
+				  struct wt_status_state *state)
 {
 	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
-
-	wt_status_get_state(&state);
-
-	if (state.merge_in_progress)
-		show_merge_in_progress(s, &state, state_color);
-	else if (state.am_in_progress)
-		show_am_in_progress(s, &state, state_color);
-	else if (state.rebase_in_progress || state.rebase_interactive_in_progress)
-		show_rebase_in_progress(s, &state, state_color);
-	else if (state.cherry_pick_in_progress)
-		show_cherry_pick_in_progress(s, &state, state_color);
-	if (state.bisect_in_progress)
-		show_bisect_in_progress(s, &state, state_color);
-	free(state.branch);
-	free(state.onto);
+	if (state->merge_in_progress)
+		show_merge_in_progress(s, state, state_color);
+	else if (state->am_in_progress)
+		show_am_in_progress(s, state, state_color);
+	else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
+		show_rebase_in_progress(s, state, state_color);
+	else if (state->cherry_pick_in_progress)
+		show_cherry_pick_in_progress(s, state, state_color);
+	if (state->bisect_in_progress)
+		show_bisect_in_progress(s, state, state_color);
 }
 
 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);
+	struct wt_status_state state;
+
+	wt_status_get_state(&state);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1087,7 +1085,10 @@ void wt_status_print(struct wt_status *s)
 			wt_status_print_tracking(s);
 	}
 
-	wt_status_print_state(s);
+	wt_status_print_state(s, &state);
+	free(state.branch);
+	free(state.onto);
+
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial commit"));
-- 
1.8.1.2.536.gf441e6d

[PATCH 4/5] status: show the ref that is checked out, even if it's detached

[Nguyễn Thái Ngọc Duy]

When a remote ref or a tag is checked out, HEAD is automatically
detached. There is no user friendly way to find out what ref is
checked out in this case. This patch digs in reflog for this
information and shows "Detached from origin/master" or "Detached from
v1.8.0" instead of "Currently not on any branch".

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 wt-status.h |  3 ++-
 2 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 6a3566b..4b6421a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1001,7 +1001,60 @@ static void read_and_strip_branch(struct strbuf *sb,
 		*branch = xstrdup(*branch);
 }
 
-void wt_status_get_state(struct wt_status_state *state)
+struct grab_1st_switch_cbdata {
+	struct strbuf buf;
+	unsigned char sha1[20];
+};
+
+static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+			   const char *email, unsigned long timestamp, int tz,
+			   const char *message, void *cb_data)
+{
+	struct grab_1st_switch_cbdata *cb = cb_data;
+	const char *target = NULL;
+
+	if (prefixcmp(message, "checkout: moving from "))
+		return 0;
+	message += strlen("checkout: moving from ");
+	target = strstr(message, " to ");
+	if (!target)
+		return 0;
+	target += strlen(" to ");
+	strbuf_reset(&cb->buf);
+	hashcpy(cb->sha1, nsha1);
+	if (!prefixcmp(target, "refs/")) {
+		const char *end = target;
+		while (*end && *end != '\n')
+			end++;
+		strbuf_add(&cb->buf, target, end - target);
+	}
+	return 0;
+}
+
+static void wt_status_get_detached_from(struct wt_status_state *state)
+{
+	struct grab_1st_switch_cbdata cb;
+	struct commit *commit;
+	unsigned char sha1[20];
+
+	strbuf_init(&cb.buf, 0);
+	for_each_recent_reflog_ent("HEAD", grab_1st_switch, 40960, &cb);
+	if (cb.buf.len &&
+	    !read_ref(cb.buf.buf, sha1) &&
+	    (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
+	    !hashcmp(cb.sha1, commit->object.sha1)) {
+		int ofs = 0;
+		if (!prefixcmp(cb.buf.buf, "refs/tags/"))
+			ofs = strlen("refs/tags/");
+		else if (!prefixcmp(cb.buf.buf, "refs/remotes/"))
+			ofs = strlen("refs/remotes/");
+		state->detached_from = xstrdup(cb.buf.buf + ofs);
+	}
+	strbuf_release(&cb.buf);
+}
+
+void wt_status_get_state(struct wt_status_state *state,
+			 int get_detached_from)
 {
 	struct strbuf branch = STRBUF_INIT;
 	struct strbuf onto = STRBUF_INIT;
@@ -1040,6 +1093,10 @@ void wt_status_get_state(struct wt_status_state *state)
 		read_and_strip_branch(&branch, &state->branch,
 				      "BISECT_START");
 	}
+
+	if (get_detached_from)
+		wt_status_get_detached_from(state);
+
 	strbuf_release(&branch);
 	strbuf_release(&onto);
 }
@@ -1066,7 +1123,8 @@ void wt_status_print(struct wt_status *s)
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
 	struct wt_status_state state;
 
-	wt_status_get_state(&state);
+	wt_status_get_state(&state,
+			    s->branch && !strcmp(s->branch, "HEAD"));
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1074,9 +1132,14 @@ void wt_status_print(struct wt_status *s)
 		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.");
+			if (state.detached_from) {
+				branch_name = state.detached_from;
+				on_what = _("Detached from ");
+			} else {
+				branch_name = "";
+				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);
@@ -1088,6 +1151,7 @@ void wt_status_print(struct wt_status *s)
 	wt_status_print_state(s, &state);
 	free(state.branch);
 	free(state.onto);
+	free(state.detached_from);
 
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
diff --git a/wt-status.h b/wt-status.h
index 5ddcbf6..74c9055 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -81,12 +81,13 @@ struct wt_status_state {
 	int bisect_in_progress;
 	char *branch;
 	char *onto;
+	char *detached_from;
 };
 
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
-void wt_status_get_state(struct wt_status_state *state);
+void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

Re: [PATCH 4/5] status: show the ref that is checked out, even if it's detached

[Junio C Hamano]

> When a remote ref or a tag is checked out, HEAD is automatically
> detached. There is no user friendly way to find out what ref is
> checked out in this case. This patch digs in reflog for this
> information and shows "Detached from origin/master" or "Detached from
> v1.8.0" instead of "Currently not on any branch".

"Detached from" is a nice attempt to compromise in the phrasing.

We usually say you detach HEAD at v1.8.0, but what is shown is what
started from such a state but then the user may have built more
history on top of it and may no longer be at v1.8.0, so obviously we
do not want to say "Detached at".  We are in a "detached at v1.8.0
and then possibly built one or more commits on top" state (would it
be helpful to differentiate the "nothing built on top" and "some
commits have been built on top" cases, I wonder).

Also I wonder if you could do a bit more to help the users who do:

    $ git checkout $(git merge-base HEAD nd/branch-show-rebase-bisect-state)

aka

    $ git checkout ...nd/branch-show-rebase-bisect-state

and then do one or more commits on top.

Instead of punting to "Currently not on any branch", would it help
to show the place you first detached at, so that the user can then
grab that commit object name and run

    $ git log --oneline $that_commit..

or something?

> +static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
> +			   const char *email, unsigned long timestamp, int tz,
> +			   const char *message, void *cb_data)
> +{
> +	struct grab_1st_switch_cbdata *cb = cb_data;
> +	const char *target = NULL;
> +
> +	if (prefixcmp(message, "checkout: moving from "))
> +		return 0;
> +	message += strlen("checkout: moving from ");
> +	target = strstr(message, " to ");
> +	if (!target)
> +		return 0;
> +	target += strlen(" to ");
> +	strbuf_reset(&cb->buf);
> +	hashcpy(cb->sha1, nsha1);
> +	if (!prefixcmp(target, "refs/")) {
> +		const char *end = target;
> +		while (*end && *end != '\n')
> +			end++;
> +		strbuf_add(&cb->buf, target, end - target);
> +	}
> +	return 0;
> +}

Can't this be done by generalizing grab_nth_branch_switch() and then
exposing it as part of the general API?  

I also feel uneasy about two issues this and the previous change
introduces:

 1) It is somewhat unnerving that this step reads what comes after
    "to", while nth_branch_switch() reads what comes between "from"
    and "to", but it probably cannot be avoided because this series
    wants to know what we switched "to" earlier, while "checkout -"
    wants to know what we switched "from"

 2) The previous one records this sequence in a funny way:

        : start from branch A
        git checkout B
        git checkout C

    The resulting reflog entries result in

        checkout: moving from A to refs/heads/B
        checkout: moving from B to refs/heads/C

    even though existing code and tools are expecting to read

        checkout: moving from A to B
        checkout: moving from B to C

By the way, even though the title of this patch is "status: show the
ref that is checked out, even if it's detached", a quick check with

        $ cd ../linux-3.0
        $ git describe
        v3.8-rc7
        $ ../git.git/git-checkout v3.8-rc7
        $ tail -n 1 .git/logs/HEAD | sed -e 's/.*checkout/checkout/'
        checkout: moving from master to refs/tags/v3.8-rc7
        $ ../git.git/git-status | head -n 1
        # Not currently on any branch.
        $ ../git.git/git-branch -v
        * (no branch) 836dc9e Linux 3.8-rc7
          master      836dc9e Linux 3.8-rc7

does not seem to give me anything more helpful.

Re: [PATCH 4/5] status: show the ref that is checked out, even if it's detached

[Duy Nguyen]

On Mon, Mar 4, 2013 at 5:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> When a remote ref or a tag is checked out, HEAD is automatically
>> detached. There is no user friendly way to find out what ref is
>> checked out in this case. This patch digs in reflog for this
>> information and shows "Detached from origin/master" or "Detached from
>> v1.8.0" instead of "Currently not on any branch".
>
> "Detached from" is a nice attempt to compromise in the phrasing.
>
> We usually say you detach HEAD at v1.8.0, but what is shown is what
> started from such a state but then the user may have built more
> history on top of it and may no longer be at v1.8.0, so obviously we
> do not want to say "Detached at".  We are in a "detached at v1.8.0
> and then possibly built one or more commits on top" state (would it
> be helpful to differentiate the "nothing built on top" and "some
> commits have been built on top" cases, I wonder).

Hmm.. never thought of that subtlety. Differentiating should be
possible. I'm just not sure if it would be helpful. Comments people,
do or not do?

> Also I wonder if you could do a bit more to help the users who do:
>
>     $ git checkout $(git merge-base HEAD nd/branch-show-rebase-bisect-state)
>
> aka
>
>     $ git checkout ...nd/branch-show-rebase-bisect-state
>
> and then do one or more commits on top.
>
> Instead of punting to "Currently not on any branch", would it help
> to show the place you first detached at, so that the user can then
> grab that commit object name and run
>
>     $ git log --oneline $that_commit..
>
> or something?

$that_commit would be HEAD@{-1} right? Should that be used instead of
grabbing random SHA-1 shown in git-status?


>> +static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
>
> Can't this be done by generalizing grab_nth_branch_switch() and then
> exposing it as part of the general API?

We could share the code, yes. Initially I wanted something that
resolves "@{-1}" and gives me the reflog entry. Maybe we could add a
function to do that.


> I also feel uneasy about two issues this and the previous change

I assume "previous change" is 1/5, reflog format change.


>  2) The previous one records this sequence in a funny way:
>
>         : start from branch A
>         git checkout B
>         git checkout C
>
>     The resulting reflog entries result in
>
>         checkout: moving from A to refs/heads/B
>         checkout: moving from B to refs/heads/C
>
>     even though existing code and tools are expecting to read
>
>         checkout: moving from A to B
>         checkout: moving from B to C

It does not record exactly user input, but the "to" part is basically
extended sha-1 and I don't think current tools parse them manually.
Instead just they should just pass them to git-rev-parse to do the
job. So this change is not really bad. But again the recent '!'
incident in attr.c shows that I know nothing about real world usage.
We could do dwim when parsing the reflog, which introduces no changes
in reflog format.


> By the way, even though the title of this patch is "status: show the
> ref that is checked out, even if it's detached", a quick check with
>
>         $ cd ../linux-3.0
>         $ git describe
>         v3.8-rc7
>         $ ../git.git/git-checkout v3.8-rc7
>         $ tail -n 1 .git/logs/HEAD | sed -e 's/.*checkout/checkout/'
>         checkout: moving from master to refs/tags/v3.8-rc7
>         $ ../git.git/git-status | head -n 1
>         # Not currently on any branch.
>         $ ../git.git/git-branch -v
>         * (no branch) 836dc9e Linux 3.8-rc7
>           master      836dc9e Linux 3.8-rc7
>
> does not seem to give me anything more helpful.

Yeah. I blame myself for copying from interpret_nth_prior_checkout()
without full understanding, and git's poor documentation (there's no
description about for_each_recent_reflog_ent, and that the caller is
supposed to call for_each_reflog_ent in some case; but I think this
convention is unintuitive, for_each_recent_reflog_ent should do that
itself)
-- 
Duy

Re: [PATCH 4/5] status: show the ref that is checked out, even if it's detached

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> Instead of punting to "Currently not on any branch", would it help
>> to show the place you first detached at, so that the user can then
>> grab that commit object name and run
>>
>>     $ git log --oneline $that_commit..
>>
>> or something?
>
> $that_commit would be HEAD@{-1} right? Should that be used instead of
> grabbing random SHA-1 shown in git-status?

You can say "git checkout @{-1}" to go back.  That is the commit you
came _from_.

But I think what you were computing for $that_commit was different.
It's "the ref that is checked out, even if it's detached".

After doing:

	: start from anywhere
        git checkout master	;# on master
        git checkout branch~4   ;# detach at that commit
        ... hack hack hack ...
        git commit; git commit

your @{-1} is 'master'; "the ref that is checked out, even if its'
detached" is 'branch~4', and that is what you want as $that_commit
in the example.  That shows your _progress_, which has no relation
with 'master' that is @{-1}.

Re: [PATCH 4/5] status: show the ref that is checked out, even if it's detached

[Duy Nguyen]

OK new try. This

- no longer requires 1/5 (i'll resend full series later when the
  wanted behavior is found)

- shows either "detached from" or "detached at". We could even do "4
  commits from detached point XXX", like we do "5 commits ahead of
  upstream". But I'm not sure if we should do that.

- otherwise shows "detached from <sha1>". IOW "currently not on any
  branch" is no longer shown.

- fixes the case when reflog is too short and
  for_each_recent_reflog_ent returns false.

I did not merge grab_1st_switch and grab_nth_branch_switch because
they use different part of the reflog and merging seems to make it
more complicated than necessary.

-- 8< --
Subject: [PATCH] status: show more info than "currently not on any branch"

When a remote ref or a tag is checked out, HEAD is automatically
detached. There is no user friendly way to find out what ref is
checked out in this case. This patch digs in reflog for this
information and shows "Detached from/at origin/master" or "Detached
from/at v1.8.0".

When it cannot figure out the original ref, it shows an abbreviated
SHA-1 instead. "Currently not on any branch" would never display.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t7406-submodule-update.sh |  6 ++--
 t/t7512-status-help.sh      | 52 +++++++++++++++++----------
 wt-status.c                 | 87 ++++++++++++++++++++++++++++++++++++++++++---
 wt-status.h                 |  4 ++-
 4 files changed, 123 insertions(+), 26 deletions(-)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 4975ec0..50ac020 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -664,8 +664,10 @@ test_expect_success 'submodule add properly re-creates deeper level submodules'
 
 test_expect_success 'submodule update properly revives a moved submodule' '
 	(cd super &&
+	 H=$(git rev-parse --short HEAD) &&
 	 git commit -am "pre move" &&
-	 git status >expect&&
+	 H2=$(git rev-parse --short HEAD) &&
+	 git status | sed "s/$H/XXX/" >expect &&
 	 H=$(cd submodule2; git rev-parse HEAD) &&
 	 git rm --cached submodule2 &&
 	 rm -rf submodule2 &&
@@ -674,7 +676,7 @@ test_expect_success 'submodule update properly revives a moved submodule' '
 	 git config -f .gitmodules submodule.submodule2.path "moved/sub module"
 	 git commit -am "post move" &&
 	 git submodule update &&
-	 git status >actual &&
+	 git status | sed "s/$H2/XXX/" >actual &&
 	 test_cmp expect actual
 	)
 '
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index d2da89a..c4f030f 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -76,7 +76,7 @@ test_expect_success 'status when rebase in progress before resolving conflicts'
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -103,7 +103,7 @@ test_expect_success 'status when rebase in progress before rebase --continue' '
 	echo three >main.txt &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -135,7 +135,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' '
 	ONTO=$(git rev-parse --short rebase_i_conflicts) &&
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -161,7 +161,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' '
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -187,9 +187,10 @@ test_expect_success 'status when rebasing -i in edit mode' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~2) &&
+	TGT=$(git rev-parse --short two_rebase_i) &&
 	git rebase -i HEAD~2 &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -214,8 +215,9 @@ test_expect_success 'status when splitting a commit' '
 	ONTO=$(git rev-parse --short HEAD~3) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
+	TGT=$(git rev-parse --short HEAD) &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached at $TGT
 	# You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -243,10 +245,11 @@ test_expect_success 'status after editing the last commit with --amend during a
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	TGT=$(git rev-parse --short three_amend) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -276,7 +279,7 @@ test_expect_success 'status: (continue first edit) second edit' '
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -298,7 +301,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -325,7 +328,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -347,7 +350,7 @@ test_expect_success 'status: (amend first edit) second edit' '
 	git commit --amend -m "a" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -370,7 +373,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -398,7 +401,7 @@ test_expect_success 'status: (amend first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "d" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -422,7 +425,7 @@ test_expect_success 'status: (split first edit) second edit' '
 	git commit -m "e" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -447,7 +450,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -477,7 +480,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "h" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -572,8 +575,9 @@ test_expect_success 'status when bisecting' '
 	git bisect start &&
 	git bisect bad &&
 	git bisect good one_bisect &&
-	cat >expected <<-\EOF &&
-	# Not currently on any branch.
+	TGT=$(git rev-parse --short two_bisect) &&
+	cat >expected <<-EOF &&
+	# Detached at $TGT
 	# You are currently bisecting branch '\''bisect'\''.
 	#   (use "git bisect reset" to get back to the original branch)
 	#
@@ -596,7 +600,7 @@ test_expect_success 'status when rebase conflicts with statushints disabled' '
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# Detached at $ONTO
 	# You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
 	#
 	# Unmerged paths:
@@ -662,5 +666,15 @@ test_expect_success 'status when cherry-picking after resolving conflicts' '
 	test_i18ncmp expected actual
 '
 
+test_expect_success 'status showing detached from a tag' '
+	test_commit atag tagging &&
+	git checkout atag &&
+	cat >expected <<-\EOF
+	# Detached at atag
+	nothing to commit (use -u to show untracked files)
+	EOF
+	git status --untracked-files=no >actual &&
+	test_i18ncmp expected actual
+'
 
 test_done
diff --git a/wt-status.c b/wt-status.c
index 6a3566b..7ad3c2b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1001,7 +1001,70 @@ static void read_and_strip_branch(struct strbuf *sb,
 		*branch = xstrdup(*branch);
 }
 
-void wt_status_get_state(struct wt_status_state *state)
+struct grab_1st_switch_cbdata {
+	struct strbuf buf;
+	unsigned char nsha1[20];
+};
+
+static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+			   const char *email, unsigned long timestamp, int tz,
+			   const char *message, void *cb_data)
+{
+	struct grab_1st_switch_cbdata *cb = cb_data;
+	const char *target = NULL, *end;
+
+	if (prefixcmp(message, "checkout: moving from "))
+		return 0;
+	message += strlen("checkout: moving from ");
+	target = strstr(message, " to ");
+	if (!target)
+		return 0;
+	target += strlen(" to ");
+	strbuf_reset(&cb->buf);
+	hashcpy(cb->nsha1, nsha1);
+	for (end = target; *end && *end != '\n'; end++)
+		;
+	strbuf_add(&cb->buf, target, end - target);
+	return 0;
+}
+
+static void wt_status_get_detached_from(struct wt_status_state *state)
+{
+	struct grab_1st_switch_cbdata cb;
+	struct commit *commit;
+	unsigned char sha1[20];
+	char *ref = NULL;
+
+	strbuf_init(&cb.buf, 0);
+	if (for_each_recent_reflog_ent("HEAD", grab_1st_switch, 4096, &cb))
+		for_each_reflog_ent("HEAD", grab_1st_switch, &cb);
+	if (!cb.buf.len)
+		return;
+
+	if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
+	    (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
+	    !hashcmp(cb.nsha1, commit->object.sha1)) {
+		int ofs;
+		if (!prefixcmp(ref, "refs/tags/"))
+			ofs = strlen("refs/tags/");
+		else if (!prefixcmp(ref, "refs/remotes/"))
+			ofs = strlen("refs/remotes/");
+		else
+			ofs = 0;
+		state->detached_from = xstrdup(ref + ofs);
+		hashcpy(state->detached_sha1, sha1);
+	} else if (!get_sha1(cb.buf.buf, sha1)) {
+		state->detached_from =
+			xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
+		hashcpy(state->detached_sha1, sha1);
+	}
+
+	free(ref);
+	strbuf_release(&cb.buf);
+}
+
+void wt_status_get_state(struct wt_status_state *state,
+			 int get_detached_from)
 {
 	struct strbuf branch = STRBUF_INIT;
 	struct strbuf onto = STRBUF_INIT;
@@ -1040,6 +1103,10 @@ void wt_status_get_state(struct wt_status_state *state)
 		read_and_strip_branch(&branch, &state->branch,
 				      "BISECT_START");
 	}
+
+	if (get_detached_from)
+		wt_status_get_detached_from(state);
+
 	strbuf_release(&branch);
 	strbuf_release(&onto);
 }
@@ -1066,7 +1133,8 @@ void wt_status_print(struct wt_status *s)
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
 	struct wt_status_state state;
 
-	wt_status_get_state(&state);
+	wt_status_get_state(&state,
+			    s->branch && !strcmp(s->branch, "HEAD"));
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1074,9 +1142,19 @@ void wt_status_print(struct wt_status *s)
 		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.");
+			if (state.detached_from) {
+				unsigned char sha1[20];
+				branch_name = state.detached_from;
+				if (!get_sha1("HEAD", sha1) &&
+				    !hashcmp(sha1, state.detached_sha1))
+					on_what = _("Detached at ");
+				else
+					on_what = _("Detached from ");
+			} else {
+				branch_name = "";
+				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);
@@ -1088,6 +1166,7 @@ void wt_status_print(struct wt_status *s)
 	wt_status_print_state(s, &state);
 	free(state.branch);
 	free(state.onto);
+	free(state.detached_from);
 
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
diff --git a/wt-status.h b/wt-status.h
index 5ddcbf6..5cb7df9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -81,12 +81,14 @@ struct wt_status_state {
 	int bisect_in_progress;
 	char *branch;
 	char *onto;
+	char *detached_from;
+	unsigned char detached_sha1[20];
 };
 
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
-void wt_status_get_state(struct wt_status_state *state);
+void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 8< --

Re: [PATCH 4/5] status: show the ref that is checked out, even if it's detached

[Matthieu Moy]

Duy Nguyen <pclouds@gmail.com> writes:

> -	# Not currently on any branch.
> +	# Detached at $ONTO

Without the context, I don't think "Detached" alone says something to
the user. "Detached HEAD at ..." would IMHO be clearer and at least give
the user enough keywords to search the web/doc for an explanation.

Actually, in the case of "Detached HEAD at $branch", you may even add an
advice saying something like:

  (run "git checkout $branch" to switch to branch $branch)

or so. But that's not the common case, so maybe it's not that useful.

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

[PATCH 5/5] branch: show more information when HEAD is detached

[Nguyễn Thái Ngọc Duy]

This prints more helpful info when HEAD is detached: is it detached
because of bisect or rebase? What is the original branch name in those
cases? Is it detached because the user checks out a remote ref or a
tag (and which one)?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c            | 25 ++++++++++++++++++++++++-
 t/t6030-bisect-porcelain.sh |  2 +-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..02dee0d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -18,6 +18,7 @@
 #include "string-list.h"
 #include "column.h"
 #include "utf8.h"
+#include "wt-status.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [options] [-r | -a] [--merged | --no-merged]"),
@@ -550,6 +551,28 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description()
+{
+	struct strbuf desc = STRBUF_INIT;
+	struct wt_status_state state;
+	wt_status_get_state(&state, 1);
+	if (state.rebase_in_progress ||
+	    state.rebase_interactive_in_progress)
+		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+			    state.branch);
+	else if (state.bisect_in_progress)
+		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
+			    state.branch);
+	else if (state.detached_from)
+		strbuf_addf(&desc, _("(detached from %s)"),
+			    state.detached_from);
+	else
+		strbuf_addstr(&desc, _("(no branch)"));
+	free(state.branch);
+	free(state.onto);
+	free(state.detached_from);
+	return strbuf_detach(&desc, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +580,7 @@ static void show_detached(struct ref_list *ref_list)
 
 	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
 		struct ref_item item;
-		item.name = xstrdup(_("(no branch)"));
+		item.name = get_head_description();
 		item.width = utf8_strwidth(item.name);
 		item.kind = REF_LOCAL_BRANCH;
 		item.dest = NULL;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 3e0e15f..9b6f0d0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	cp .git/BISECT_START saved &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	test_i18ngrep "* (no branch)" branch.output > /dev/null &&
+	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
 	test_cmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
-- 
1.8.1.2.536.gf441e6d

Re: [PATCH 5/5] branch: show more information when HEAD is detached

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This prints more helpful info when HEAD is detached: is it detached
> because of bisect or rebase? What is the original branch name in those
> cases? Is it detached because the user checks out a remote ref or a
> tag (and which one)?
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/branch.c            | 25 ++++++++++++++++++++++++-
>  t/t6030-bisect-porcelain.sh |  2 +-
>  2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 6371bf9..02dee0d 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -18,6 +18,7 @@
>  #include "string-list.h"
>  #include "column.h"
>  #include "utf8.h"
> +#include "wt-status.h"
>  
>  static const char * const builtin_branch_usage[] = {
>  	N_("git branch [options] [-r | -a] [--merged | --no-merged]"),
> @@ -550,6 +551,28 @@ static int calc_maxwidth(struct ref_list *refs)
>  	return w;
>  }
>  
> +static char *get_head_description()

s/()/(void)/;

I think the series is much easier to read than the previous round
(except for some nits, which I'll send separately).  We would want
to add tests to protect the "where did we detach from" feature done
by [PATCH 4/5], which does not seem to have any.

[PATCH v2 0/4] nd/branch-show-rebase-bisect-state updates

[Nguyễn Thái Ngọc Duy]

Since the previous round:

 - reflog format change is dropped
 - almost always show "HEAD detached from/at xxx" instead of
   "currently not on any branch", where "xxx" is either a ref, or an
   abbreviated sha-1

I wanted to introduce something like @{-1:to} that resolves the "to"
sha-1 in reflog, in addition to @{-1} which resolves the "from" part.
But the syntax looks ugly so I dropped it. Meanwhile people could do

git log -1 -g --grep-reflog checkout --pretty=format:%h

to retrieve the same info.

Nguyễn Thái Ngọc Duy (4):
  wt-status: split wt_status_state parsing function out
  wt-status: move wt_status_get_state() out to wt_status_print()
  status: show more info than "currently not on any branch"
  branch: show more information when HEAD is detached

 builtin/branch.c            |  25 +++++++-
 t/t3203-branch-output.sh    |   6 +-
 t/t6030-bisect-porcelain.sh |   2 +-
 t/t7406-submodule-update.sh |   6 +-
 t/t7512-status-help.sh      |  52 +++++++++------
 wt-status.c                 | 152 ++++++++++++++++++++++++++++++++++----------
 wt-status.h                 |   7 +-
 7 files changed, 190 insertions(+), 60 deletions(-)

-- 
1.8.1.2.536.gf441e6d

[PATCH v2 1/4] wt-status: split wt_status_state parsing function out

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 52 +++++++++++++++++++++++++++++++---------------------
 wt-status.h |  5 +++--
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index ef405d0..183aafe 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -970,7 +970,7 @@ static void show_bisect_in_progress(struct wt_status *s,
  * Extract branch information from rebase/bisect
  */
 static void read_and_strip_branch(struct strbuf *sb,
-				  const char **branch,
+				  char **branch,
 				  const char *path)
 {
 	unsigned char sha1[20];
@@ -994,52 +994,62 @@ static void read_and_strip_branch(struct strbuf *sb,
 		strbuf_addstr(sb, abbrev);
 		*branch = sb->buf;
 	} else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
-		;
+		*branch = NULL;
 	else			/* bisect */
 		*branch = sb->buf;
+	if (*branch)
+		*branch = xstrdup(*branch);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+void wt_status_get_state(struct wt_status_state *state)
 {
-	const char *state_color = color(WT_STATUS_HEADER, s);
 	struct strbuf branch = STRBUF_INIT;
 	struct strbuf onto = STRBUF_INIT;
-	struct wt_status_state state;
 	struct stat st;
 
-	memset(&state, 0, sizeof(state));
+	memset(state, 0, sizeof(*state));
 
 	if (!stat(git_path("MERGE_HEAD"), &st)) {
-		state.merge_in_progress = 1;
+		state->merge_in_progress = 1;
 	} else if (!stat(git_path("rebase-apply"), &st)) {
 		if (!stat(git_path("rebase-apply/applying"), &st)) {
-			state.am_in_progress = 1;
+			state->am_in_progress = 1;
 			if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
-				state.am_empty_patch = 1;
+				state->am_empty_patch = 1;
 		} else {
-			state.rebase_in_progress = 1;
-			read_and_strip_branch(&branch, &state.branch,
+			state->rebase_in_progress = 1;
+			read_and_strip_branch(&branch, &state->branch,
 					      "rebase-apply/head-name");
-			read_and_strip_branch(&onto, &state.onto,
+			read_and_strip_branch(&onto, &state->onto,
 					      "rebase-apply/onto");
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
-			state.rebase_interactive_in_progress = 1;
+			state->rebase_interactive_in_progress = 1;
 		else
-			state.rebase_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
+			state->rebase_in_progress = 1;
+		read_and_strip_branch(&branch, &state->branch,
 				      "rebase-merge/head-name");
-		read_and_strip_branch(&onto, &state.onto,
+		read_and_strip_branch(&onto, &state->onto,
 				      "rebase-merge/onto");
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
-		state.cherry_pick_in_progress = 1;
+		state->cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st)) {
-		state.bisect_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
+		state->bisect_in_progress = 1;
+		read_and_strip_branch(&branch, &state->branch,
 				      "BISECT_START");
 	}
+	strbuf_release(&branch);
+	strbuf_release(&onto);
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+	const char *state_color = color(WT_STATUS_HEADER, s);
+	struct wt_status_state state;
+
+	wt_status_get_state(&state);
 
 	if (state.merge_in_progress)
 		show_merge_in_progress(s, &state, state_color);
@@ -1051,8 +1061,8 @@ static void wt_status_print_state(struct wt_status *s)
 		show_cherry_pick_in_progress(s, &state, state_color);
 	if (state.bisect_in_progress)
 		show_bisect_in_progress(s, &state, state_color);
-	strbuf_release(&branch);
-	strbuf_release(&onto);
+	free(state.branch);
+	free(state.onto);
 }
 
 void wt_status_print(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 81e1dcf..5ddcbf6 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,13 +79,14 @@ struct wt_status_state {
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
-	const char *branch;
-	const char *onto;
+	char *branch;
+	char *onto;
 };
 
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
+void wt_status_get_state(struct wt_status_state *state);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

Re: [PATCH v2 1/4] wt-status: split wt_status_state parsing function out

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  wt-status.c | 52 +++++++++++++++++++++++++++++++---------------------
>  wt-status.h |  5 +++--
>  2 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index ef405d0..183aafe 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -970,7 +970,7 @@ static void show_bisect_in_progress(struct wt_status *s,
>   * Extract branch information from rebase/bisect
>   */
>  static void read_and_strip_branch(struct strbuf *sb,
> -				  const char **branch,
> +				  char **branch,
>  				  const char *path)
>  {
>  	unsigned char sha1[20];
> @@ -994,52 +994,62 @@ static void read_and_strip_branch(struct strbuf *sb,
>  		strbuf_addstr(sb, abbrev);
>  		*branch = sb->buf;
>  	} else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
> -		;
> +		*branch = NULL;
>  	else			/* bisect */
>  		*branch = sb->buf;
> +	if (*branch)
> +		*branch = xstrdup(*branch);
>  }

The reason why the original print_state() kept two strbufs in it was
because its use of the return value (in *branch) from this function
was private and it did not want to have to strdup anything.

With this change, I suspect that it is much saner to make this
function *not* take any external strbuf as input, because you are
always returning a piece of memory that belongs to the caller, or a
NULL.

In other words, with the new world order, wouldn't a saner function
signature be:

	static const char *read_and_strip_branch(const char **path);

after this patch?

Also I notice that the error-return cases of this function may be
wrong even before your patch.  Shouldn't it be doing *branch = NULL
(and 'return NULL' after the suggested change)?

Re: [PATCH v2 1/4] wt-status: split wt_status_state parsing function out

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> In other words, with the new world order, wouldn't a saner function
> signature be:
>
> 	static const char *read_and_strip_branch(const char **path);

Eh, discard an asterisk from there.  "Given a string to name the
file, read it to find the branch name recorded in it".

> after this patch?
>
> Also I notice that the error-return cases of this function may be
> wrong even before your patch.  Shouldn't it be doing *branch = NULL
> (and 'return NULL' after the suggested change)?

[PATCH v2 2/4] wt-status: move wt_status_get_state() out to wt_status_print()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 183aafe..6a3566b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1044,31 +1044,29 @@ void wt_status_get_state(struct wt_status_state *state)
 	strbuf_release(&onto);
 }
 
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_print_state(struct wt_status *s,
+				  struct wt_status_state *state)
 {
 	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
-
-	wt_status_get_state(&state);
-
-	if (state.merge_in_progress)
-		show_merge_in_progress(s, &state, state_color);
-	else if (state.am_in_progress)
-		show_am_in_progress(s, &state, state_color);
-	else if (state.rebase_in_progress || state.rebase_interactive_in_progress)
-		show_rebase_in_progress(s, &state, state_color);
-	else if (state.cherry_pick_in_progress)
-		show_cherry_pick_in_progress(s, &state, state_color);
-	if (state.bisect_in_progress)
-		show_bisect_in_progress(s, &state, state_color);
-	free(state.branch);
-	free(state.onto);
+	if (state->merge_in_progress)
+		show_merge_in_progress(s, state, state_color);
+	else if (state->am_in_progress)
+		show_am_in_progress(s, state, state_color);
+	else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
+		show_rebase_in_progress(s, state, state_color);
+	else if (state->cherry_pick_in_progress)
+		show_cherry_pick_in_progress(s, state, state_color);
+	if (state->bisect_in_progress)
+		show_bisect_in_progress(s, state, state_color);
 }
 
 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);
+	struct wt_status_state state;
+
+	wt_status_get_state(&state);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1087,7 +1085,10 @@ void wt_status_print(struct wt_status *s)
 			wt_status_print_tracking(s);
 	}
 
-	wt_status_print_state(s);
+	wt_status_print_state(s, &state);
+	free(state.branch);
+	free(state.onto);
+
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial commit"));
-- 
1.8.1.2.536.gf441e6d

[PATCH v2 3/4] status: show more info than "currently not on any branch"

[Nguyễn Thái Ngọc Duy]

When a remote ref or a tag is checked out, HEAD is automatically
detached. There is no user-friendly way to find out what ref is
checked out in this case. This patch digs in reflog for this
information and shows "HEAD detached from origin/master" or "HEAD
detached at v1.8.0" instead of "currently not on any branch".

When it cannot figure out the original ref, it shows an abbreviated
SHA-1. "Currently not on any branch" would never display (unless
reflog is pruned to near empty that the last checkout entry is lost).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t7406-submodule-update.sh |  6 ++--
 t/t7512-status-help.sh      | 52 +++++++++++++++++----------
 wt-status.c                 | 85 ++++++++++++++++++++++++++++++++++++++++++---
 wt-status.h                 |  4 ++-
 4 files changed, 121 insertions(+), 26 deletions(-)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 4975ec0..50ac020 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -664,8 +664,10 @@ test_expect_success 'submodule add properly re-creates deeper level submodules'
 
 test_expect_success 'submodule update properly revives a moved submodule' '
 	(cd super &&
+	 H=$(git rev-parse --short HEAD) &&
 	 git commit -am "pre move" &&
-	 git status >expect&&
+	 H2=$(git rev-parse --short HEAD) &&
+	 git status | sed "s/$H/XXX/" >expect &&
 	 H=$(cd submodule2; git rev-parse HEAD) &&
 	 git rm --cached submodule2 &&
 	 rm -rf submodule2 &&
@@ -674,7 +676,7 @@ test_expect_success 'submodule update properly revives a moved submodule' '
 	 git config -f .gitmodules submodule.submodule2.path "moved/sub module"
 	 git commit -am "post move" &&
 	 git submodule update &&
-	 git status >actual &&
+	 git status | sed "s/$H2/XXX/" >actual &&
 	 test_cmp expect actual
 	)
 '
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index d2da89a..da22088 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -76,7 +76,7 @@ test_expect_success 'status when rebase in progress before resolving conflicts'
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -103,7 +103,7 @@ test_expect_success 'status when rebase in progress before rebase --continue' '
 	echo three >main.txt &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -135,7 +135,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' '
 	ONTO=$(git rev-parse --short rebase_i_conflicts) &&
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -161,7 +161,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' '
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -187,9 +187,10 @@ test_expect_success 'status when rebasing -i in edit mode' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~2) &&
+	TGT=$(git rev-parse --short two_rebase_i) &&
 	git rebase -i HEAD~2 &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -214,8 +215,9 @@ test_expect_success 'status when splitting a commit' '
 	ONTO=$(git rev-parse --short HEAD~3) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
+	TGT=$(git rev-parse --short HEAD) &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $TGT
 	# You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -243,10 +245,11 @@ test_expect_success 'status after editing the last commit with --amend during a
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	TGT=$(git rev-parse --short three_amend) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -276,7 +279,7 @@ test_expect_success 'status: (continue first edit) second edit' '
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -298,7 +301,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -325,7 +328,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -347,7 +350,7 @@ test_expect_success 'status: (amend first edit) second edit' '
 	git commit --amend -m "a" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -370,7 +373,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -398,7 +401,7 @@ test_expect_success 'status: (amend first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "d" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -422,7 +425,7 @@ test_expect_success 'status: (split first edit) second edit' '
 	git commit -m "e" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -447,7 +450,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -477,7 +480,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "h" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -572,8 +575,9 @@ test_expect_success 'status when bisecting' '
 	git bisect start &&
 	git bisect bad &&
 	git bisect good one_bisect &&
-	cat >expected <<-\EOF &&
-	# Not currently on any branch.
+	TGT=$(git rev-parse --short two_bisect) &&
+	cat >expected <<-EOF &&
+	# HEAD detached at $TGT
 	# You are currently bisecting branch '\''bisect'\''.
 	#   (use "git bisect reset" to get back to the original branch)
 	#
@@ -596,7 +600,7 @@ test_expect_success 'status when rebase conflicts with statushints disabled' '
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
 	#
 	# Unmerged paths:
@@ -662,5 +666,15 @@ test_expect_success 'status when cherry-picking after resolving conflicts' '
 	test_i18ncmp expected actual
 '
 
+test_expect_success 'status showing detached from a tag' '
+	test_commit atag tagging &&
+	git checkout atag &&
+	cat >expected <<-\EOF
+	# HEAD detached at atag
+	nothing to commit (use -u to show untracked files)
+	EOF
+	git status --untracked-files=no >actual &&
+	test_i18ncmp expected actual
+'
 
 test_done
diff --git a/wt-status.c b/wt-status.c
index 6a3566b..7797ccf 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1001,7 +1001,68 @@ static void read_and_strip_branch(struct strbuf *sb,
 		*branch = xstrdup(*branch);
 }
 
-void wt_status_get_state(struct wt_status_state *state)
+struct grab_1st_switch_cbdata {
+	struct strbuf buf;
+	unsigned char nsha1[20];
+};
+
+static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+			   const char *email, unsigned long timestamp, int tz,
+			   const char *message, void *cb_data)
+{
+	struct grab_1st_switch_cbdata *cb = cb_data;
+	const char *target = NULL, *end;
+
+	if (prefixcmp(message, "checkout: moving from "))
+		return 0;
+	message += strlen("checkout: moving from ");
+	target = strstr(message, " to ");
+	if (!target)
+		return 0;
+	target += strlen(" to ");
+	strbuf_reset(&cb->buf);
+	hashcpy(cb->nsha1, nsha1);
+	for (end = target; *end && *end != '\n'; end++)
+		;
+	strbuf_add(&cb->buf, target, end - target);
+	return 0;
+}
+
+static void wt_status_get_detached_from(struct wt_status_state *state)
+{
+	struct grab_1st_switch_cbdata cb;
+	struct commit *commit;
+	unsigned char sha1[20];
+	char *ref = NULL;
+
+	strbuf_init(&cb.buf, 0);
+	if (for_each_recent_reflog_ent("HEAD", grab_1st_switch, 4096, &cb))
+		for_each_reflog_ent("HEAD", grab_1st_switch, &cb);
+	if (!cb.buf.len)
+		return;
+
+	if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
+	    (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
+	    !hashcmp(cb.nsha1, commit->object.sha1)) {
+		int ofs;
+		if (!prefixcmp(ref, "refs/tags/"))
+			ofs = strlen("refs/tags/");
+		else if (!prefixcmp(ref, "refs/remotes/"))
+			ofs = strlen("refs/remotes/");
+		else
+			ofs = 0;
+		state->detached_from = xstrdup(ref + ofs);
+	} else
+		state->detached_from =
+			xstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));
+	hashcpy(state->detached_sha1, cb.nsha1);
+
+	free(ref);
+	strbuf_release(&cb.buf);
+}
+
+void wt_status_get_state(struct wt_status_state *state,
+			 int get_detached_from)
 {
 	struct strbuf branch = STRBUF_INIT;
 	struct strbuf onto = STRBUF_INIT;
@@ -1040,6 +1101,10 @@ void wt_status_get_state(struct wt_status_state *state)
 		read_and_strip_branch(&branch, &state->branch,
 				      "BISECT_START");
 	}
+
+	if (get_detached_from)
+		wt_status_get_detached_from(state);
+
 	strbuf_release(&branch);
 	strbuf_release(&onto);
 }
@@ -1066,7 +1131,8 @@ void wt_status_print(struct wt_status *s)
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
 	struct wt_status_state state;
 
-	wt_status_get_state(&state);
+	wt_status_get_state(&state,
+			    s->branch && !strcmp(s->branch, "HEAD"));
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1074,9 +1140,19 @@ void wt_status_print(struct wt_status *s)
 		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.");
+			if (state.detached_from) {
+				unsigned char sha1[20];
+				branch_name = state.detached_from;
+				if (!get_sha1("HEAD", sha1) &&
+				    !hashcmp(sha1, state.detached_sha1))
+					on_what = _("HEAD detached at ");
+				else
+					on_what = _("HEAD detached from ");
+			} else {
+				branch_name = "";
+				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);
@@ -1088,6 +1164,7 @@ void wt_status_print(struct wt_status *s)
 	wt_status_print_state(s, &state);
 	free(state.branch);
 	free(state.onto);
+	free(state.detached_from);
 
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
diff --git a/wt-status.h b/wt-status.h
index 5ddcbf6..5cb7df9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -81,12 +81,14 @@ struct wt_status_state {
 	int bisect_in_progress;
 	char *branch;
 	char *onto;
+	char *detached_from;
+	unsigned char detached_sha1[20];
 };
 
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
-void wt_status_get_state(struct wt_status_state *state);
+void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

Re: [PATCH v2 3/4] status: show more info than "currently not on any branch"

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> +static void wt_status_get_detached_from(struct wt_status_state *state)
> +{
> +	struct grab_1st_switch_cbdata cb;
> +	struct commit *commit;
> +	unsigned char sha1[20];
> +	char *ref = NULL;
> +
> +	strbuf_init(&cb.buf, 0);
> +	if (for_each_recent_reflog_ent("HEAD", grab_1st_switch, 4096, &cb))
> +		for_each_reflog_ent("HEAD", grab_1st_switch, &cb);
> +	if (!cb.buf.len)
> +		return;

Is this correct?  What if the recent entries (i.e. the tail 4k) of
the HEAD reflog did not have *any* checkout?  Your callback never
returns non-zero, so as long as the HEAD reflog is sufficiently
long, for_each_recent_reflog_ent() will return 0 to signal success,
and you do not dig deeper by retrying the full reflog for HEAD,
missing the checkout that exists before the final 4k, no?

It should be more like this, I would think:

	for_each_recent_reflog_ent();
        if (!found)
        	for_each_reflog_ent();
	if (!found)
        	return;

Using cb.buf.len as the "found" marker may be correct, but I found
it a bit subtle to my taste, without explanation.  Adding an
explicit bit to "struct grab_1st_switch_cbdata" would be cleaner and
more resistant to future changes, I think.

> +
> +	if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
> +	    (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
> +	    !hashcmp(cb.nsha1, commit->object.sha1)) {

That feels unnecessarily expensive.  Why not hashcmp before checking
the type of the object to reject the case where the ref moved since
the last checkout early?

For that matter, does this even need to check the type of the object
that currently sits at the ref?  Isn't it sufficient to reject this
case by seeing if sha1 is the same as cb.nsha1?

Re: [PATCH v2 3/4] status: show more info than "currently not on any branch"

[Duy Nguyen]

On Thu, Mar 7, 2013 at 2:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> +static void wt_status_get_detached_from(struct wt_status_state *state)
>> +{
>> +     struct grab_1st_switch_cbdata cb;
>> +     struct commit *commit;
>> +     unsigned char sha1[20];
>> +     char *ref = NULL;
>> +
>> +     strbuf_init(&cb.buf, 0);
>> +     if (for_each_recent_reflog_ent("HEAD", grab_1st_switch, 4096, &cb))
>> +             for_each_reflog_ent("HEAD", grab_1st_switch, &cb);
>> +     if (!cb.buf.len)
>> +             return;
>
> Is this correct?  What if the recent entries (i.e. the tail 4k) of
> the HEAD reflog did not have *any* checkout?  Your callback never
> returns non-zero, so as long as the HEAD reflog is sufficiently
> long, for_each_recent_reflog_ent() will return 0 to signal success,
> and you do not dig deeper by retrying the full reflog for HEAD,
> missing the checkout that exists before the final 4k, no?
>
> It should be more like this, I would think:
>
>         for_each_recent_reflog_ent();
>         if (!found)
>                 for_each_reflog_ent();
>         if (!found)
>                 return;

Yes. This "recent" optimization is tricky.

> Using cb.buf.len as the "found" marker may be correct, but I found
> it a bit subtle to my taste, without explanation.  Adding an
> explicit bit to "struct grab_1st_switch_cbdata" would be cleaner and
> more resistant to future changes, I think.

OK

>
>> +
>> +     if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
>> +         (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
>> +         !hashcmp(cb.nsha1, commit->object.sha1)) {
>
> That feels unnecessarily expensive.  Why not hashcmp before checking
> the type of the object to reject the case where the ref moved since
> the last checkout early?
>
> For that matter, does this even need to check the type of the object
> that currently sits at the ref?  Isn't it sufficient to reject this
> case by seeing if sha1 is the same as cb.nsha1?

nsha1 is always a commit sha-1, sha-1 could be a tag sha-1 that refers
to the same commit. hashcmp before lookup is a good idea. Although I
don't think it's expensive in the big picture. When HEAD is not
detached, we show "<n> commits ahead of @{u}" which is way more
expensive than this. As long as "git status" on detached HEAD does not
use up all the time that "git status" on branches normally does, I
think we're fine.
-- 
Duy

Re: [PATCH v2 3/4] status: show more info than "currently not on any branch"

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> It should be more like this, I would think:
>>
>>         for_each_recent_reflog_ent();
>>         if (!found)
>>                 for_each_reflog_ent();
>>         if (!found)
>>                 return;
>
> Yes. This "recent" optimization is tricky.

Not really.  What is tricky is that reflog is an append-only file
and we only have an API to let us read it in the oldest to newer
order, which is natural for the file format, but unsuited for the
purpose of finding out nth most recent anything.

See the other thread I am going to send out soon on this.

[PATCH v2 4/4] branch: show more information when HEAD is detached

[Nguyễn Thái Ngọc Duy]

This prints more helpful info when HEAD is detached: is it detached
because of bisect or rebase? What is the original branch name in those
cases? Is it detached because the user checks out a remote ref or a
tag (and which one)?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c            | 25 ++++++++++++++++++++++++-
 t/t3203-branch-output.sh    |  6 +++---
 t/t6030-bisect-porcelain.sh |  2 +-
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..02dee0d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -18,6 +18,7 @@
 #include "string-list.h"
 #include "column.h"
 #include "utf8.h"
+#include "wt-status.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [options] [-r | -a] [--merged | --no-merged]"),
@@ -550,6 +551,28 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description()
+{
+	struct strbuf desc = STRBUF_INIT;
+	struct wt_status_state state;
+	wt_status_get_state(&state, 1);
+	if (state.rebase_in_progress ||
+	    state.rebase_interactive_in_progress)
+		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+			    state.branch);
+	else if (state.bisect_in_progress)
+		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
+			    state.branch);
+	else if (state.detached_from)
+		strbuf_addf(&desc, _("(detached from %s)"),
+			    state.detached_from);
+	else
+		strbuf_addstr(&desc, _("(no branch)"));
+	free(state.branch);
+	free(state.onto);
+	free(state.detached_from);
+	return strbuf_detach(&desc, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +580,7 @@ static void show_detached(struct ref_list *ref_list)
 
 	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
 		struct ref_item item;
-		item.name = xstrdup(_("(no branch)"));
+		item.name = get_head_description();
 		item.width = utf8_strwidth(item.name);
 		item.kind = REF_LOCAL_BRANCH;
 		item.dest = NULL;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 76fe7e0..ba4f98e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -94,13 +94,13 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
 '
 
-cat >expect <<'EOF'
-* (no branch)
+test_expect_success 'git branch shows detached HEAD properly' '
+	cat >expect <<EOF &&
+* (detached from $(git rev-parse --short HEAD^0))
   branch-one
   branch-two
   master
 EOF
-test_expect_success 'git branch shows detached HEAD properly' '
 	git checkout HEAD^0 &&
 	git branch >actual &&
 	test_i18ncmp expect actual
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 3e0e15f..9b6f0d0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	cp .git/BISECT_START saved &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	test_i18ngrep "* (no branch)" branch.output > /dev/null &&
+	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
 	test_cmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
-- 
1.8.1.2.536.gf441e6d

[PATCH v2 0/4] nd/branch-show-rebase-bisect-state updates

[Nguyễn Thái Ngọc Duy]

Since the previous round:

 - reflog format change is dropped
 - almost always show "HEAD detached from/at xxx" instead of
   "currently not on any branch", where "xxx" is either a ref, or an
   abbreviated sha-1

I wanted to introduce something like @{-1:to} that resolves the "to"
sha-1 in reflog, in addition to @{-1} which resolves the "from" part.
But the syntax looks ugly so I dropped it. Meanwhile people could do

git log -1 -g --grep-reflog checkout --pretty=format:%h

to retrieve the same info.

Nguyễn Thái Ngọc Duy (4):
  wt-status: split wt_status_state parsing function out
  wt-status: move wt_status_get_state() out to wt_status_print()
  status: show more info than "currently not on any branch"
  branch: show more information when HEAD is detached

 builtin/branch.c            |  25 +++++++-
 t/t3203-branch-output.sh    |   6 +-
 t/t6030-bisect-porcelain.sh |   2 +-
 t/t7406-submodule-update.sh |   6 +-
 t/t7512-status-help.sh      |  52 +++++++++------
 wt-status.c                 | 152 ++++++++++++++++++++++++++++++++++----------
 wt-status.h                 |   7 +-
 7 files changed, 190 insertions(+), 60 deletions(-)

-- 
1.8.1.2.536.gf441e6d

[PATCH v2+ 4/4] branch: show more information when HEAD is detached

[Nguyễn Thái Ngọc Duy]

This prints more helpful info when HEAD is detached: is it detached
because of bisect or rebase? What is the original branch name in those
cases? Is it detached because the user checks out a remote ref or a
tag (and which one)?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Oops, the previous one did not do

  s/\(get_head_description\)()$/\1(void)/
 
 This one does.

 builtin/branch.c            | 25 ++++++++++++++++++++++++-
 t/t3203-branch-output.sh    |  6 +++---
 t/t6030-bisect-porcelain.sh |  2 +-
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..02dee0d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -18,6 +18,7 @@
 #include "string-list.h"
 #include "column.h"
 #include "utf8.h"
+#include "wt-status.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [options] [-r | -a] [--merged | --no-merged]"),
@@ -550,6 +551,28 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description(void)
+{
+	struct strbuf desc = STRBUF_INIT;
+	struct wt_status_state state;
+	wt_status_get_state(&state, 1);
+	if (state.rebase_in_progress ||
+	    state.rebase_interactive_in_progress)
+		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+			    state.branch);
+	else if (state.bisect_in_progress)
+		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
+			    state.branch);
+	else if (state.detached_from)
+		strbuf_addf(&desc, _("(detached from %s)"),
+			    state.detached_from);
+	else
+		strbuf_addstr(&desc, _("(no branch)"));
+	free(state.branch);
+	free(state.onto);
+	free(state.detached_from);
+	return strbuf_detach(&desc, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +580,7 @@ static void show_detached(struct ref_list *ref_list)
 
 	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
 		struct ref_item item;
-		item.name = xstrdup(_("(no branch)"));
+		item.name = get_head_description();
 		item.width = utf8_strwidth(item.name);
 		item.kind = REF_LOCAL_BRANCH;
 		item.dest = NULL;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 76fe7e0..ba4f98e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -94,13 +94,13 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
 '
 
-cat >expect <<'EOF'
-* (no branch)
+test_expect_success 'git branch shows detached HEAD properly' '
+	cat >expect <<EOF &&
+* (detached from $(git rev-parse --short HEAD^0))
   branch-one
   branch-two
   master
 EOF
-test_expect_success 'git branch shows detached HEAD properly' '
 	git checkout HEAD^0 &&
 	git branch >actual &&
 	test_i18ncmp expect actual
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 3e0e15f..9b6f0d0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	cp .git/BISECT_START saved &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	test_i18ngrep "* (no branch)" branch.output > /dev/null &&
+	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
 	test_cmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
-- 
1.8.1.2.536.gf441e6d

[PATCH v3 0/5] nd/branch-show-rebase-bisect-state updates

[Nguyễn Thái Ngọc Duy]

This round fixes the design issue of read_and_strip_branch(), makes
use of jc/reflog-reverse-walk and performs cheaper sha-1 check on
detached commits.

Nguyễn Thái Ngọc Duy (5):
  wt-status: move strbuf into read_and_strip_branch()
  wt-status: split wt_status_state parsing function out
  wt-status: move wt_status_get_state() out to wt_status_print()
  status: show more info than "currently not on any branch"
  branch: show more information when HEAD is detached

 builtin/branch.c            |  26 +++++-
 t/t3203-branch-output.sh    |   6 +-
 t/t6030-bisect-porcelain.sh |   2 +-
 t/t7406-submodule-update.sh |   6 +-
 t/t7512-status-help.sh      |  52 ++++++-----
 wt-status.c                 | 204 +++++++++++++++++++++++++++++++-------------
 wt-status.h                 |   7 +-
 7 files changed, 214 insertions(+), 89 deletions(-)

-- 
1.8.1.2.536.gf441e6d

[PATCH v3 1/5] wt-status: move strbuf into read_and_strip_branch()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 65 ++++++++++++++++++++++++++++---------------------------------
 wt-status.h |  4 ++--
 2 files changed, 32 insertions(+), 37 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index ef405d0..6cac27b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -969,41 +969,41 @@ static void show_bisect_in_progress(struct wt_status *s,
 /*
  * Extract branch information from rebase/bisect
  */
-static void read_and_strip_branch(struct strbuf *sb,
-				  const char **branch,
-				  const char *path)
+static char *read_and_strip_branch(const char *path)
 {
+	struct strbuf sb = STRBUF_INIT;
 	unsigned char sha1[20];
 
-	strbuf_reset(sb);
-	if (strbuf_read_file(sb, git_path("%s", path), 0) <= 0)
-		return;
+	if (strbuf_read_file(&sb, git_path("%s", path), 0) <= 0)
+		goto got_nothing;
 
-	while (sb->len && sb->buf[sb->len - 1] == '\n')
-		strbuf_setlen(sb, sb->len - 1);
-	if (!sb->len)
-		return;
-	if (!prefixcmp(sb->buf, "refs/heads/"))
-		*branch = sb->buf + strlen("refs/heads/");
-	else if (!prefixcmp(sb->buf, "refs/"))
-		*branch = sb->buf;
-	else if (!get_sha1_hex(sb->buf, sha1)) {
+	while (&sb.len && sb.buf[sb.len - 1] == '\n')
+		strbuf_setlen(&sb, sb.len - 1);
+	if (!sb.len)
+		goto got_nothing;
+	if (!prefixcmp(sb.buf, "refs/heads/"))
+		strbuf_remove(&sb,0, strlen("refs/heads/"));
+	else if (!prefixcmp(sb.buf, "refs/"))
+		;
+	else if (!get_sha1_hex(sb.buf, sha1)) {
 		const char *abbrev;
 		abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
-		strbuf_reset(sb);
-		strbuf_addstr(sb, abbrev);
-		*branch = sb->buf;
-	} else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
-		;
+		strbuf_reset(&sb);
+		strbuf_addstr(&sb, abbrev);
+	} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
+		goto got_nothing;
 	else			/* bisect */
-		*branch = sb->buf;
+		;
+	return strbuf_detach(&sb, NULL);
+
+got_nothing:
+	strbuf_release(&sb);
+	return NULL;
 }
 
 static void wt_status_print_state(struct wt_status *s)
 {
 	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct strbuf branch = STRBUF_INIT;
-	struct strbuf onto = STRBUF_INIT;
 	struct wt_status_state state;
 	struct stat st;
 
@@ -1018,27 +1018,22 @@ static void wt_status_print_state(struct wt_status *s)
 				state.am_empty_patch = 1;
 		} else {
 			state.rebase_in_progress = 1;
-			read_and_strip_branch(&branch, &state.branch,
-					      "rebase-apply/head-name");
-			read_and_strip_branch(&onto, &state.onto,
-					      "rebase-apply/onto");
+			state.branch = read_and_strip_branch("rebase-apply/head-name");
+			state.onto = read_and_strip_branch("rebase-apply/onto");
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
 			state.rebase_interactive_in_progress = 1;
 		else
 			state.rebase_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
-				      "rebase-merge/head-name");
-		read_and_strip_branch(&onto, &state.onto,
-				      "rebase-merge/onto");
+		state.branch = read_and_strip_branch("rebase-merge/head-name");
+		state.onto = read_and_strip_branch("rebase-merge/onto");
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
 		state.cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st)) {
 		state.bisect_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
-				      "BISECT_START");
+		state.branch = read_and_strip_branch("BISECT_START");
 	}
 
 	if (state.merge_in_progress)
@@ -1051,8 +1046,8 @@ static void wt_status_print_state(struct wt_status *s)
 		show_cherry_pick_in_progress(s, &state, state_color);
 	if (state.bisect_in_progress)
 		show_bisect_in_progress(s, &state, state_color);
-	strbuf_release(&branch);
-	strbuf_release(&onto);
+	free(state.branch);
+	free(state.onto);
 }
 
 void wt_status_print(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 81e1dcf..b8c3512 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,8 +79,8 @@ struct wt_status_state {
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
-	const char *branch;
-	const char *onto;
+	char *branch;
+	char *onto;
 };
 
 void wt_status_prepare(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

Re: [PATCH v3 1/5] wt-status: move strbuf into read_and_strip_branch()

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

You do not have anything to say above the S-o-b line?  The primary
effect of this patch is to change the lifespan and ownership rule of
the piece of memory used for state.branch/state.onto and moving the
strbuf is a mere logical consequence of that change.  It is not
explained why this change is a good idea.

> ---
>  wt-status.c | 65 ++++++++++++++++++++++++++++---------------------------------
>  wt-status.h |  4 ++--
>  2 files changed, 32 insertions(+), 37 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index ef405d0..6cac27b 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -969,41 +969,41 @@ static void show_bisect_in_progress(struct wt_status *s,
>  /*
>   * Extract branch information from rebase/bisect
>   */
> -static void read_and_strip_branch(struct strbuf *sb,
> -				  const char **branch,
> -				  const char *path)
> +static char *read_and_strip_branch(const char *path)
>  {
> +	struct strbuf sb = STRBUF_INIT;
>  	unsigned char sha1[20];
>  
> -	strbuf_reset(sb);
> -	if (strbuf_read_file(sb, git_path("%s", path), 0) <= 0)
> -		return;
> +	if (strbuf_read_file(&sb, git_path("%s", path), 0) <= 0)
> +		goto got_nothing;
>  
> -	while (sb->len && sb->buf[sb->len - 1] == '\n')
> -		strbuf_setlen(sb, sb->len - 1);
> -	if (!sb->len)
> -		return;
> -	if (!prefixcmp(sb->buf, "refs/heads/"))
> -		*branch = sb->buf + strlen("refs/heads/");
> -	else if (!prefixcmp(sb->buf, "refs/"))
> -		*branch = sb->buf;
> -	else if (!get_sha1_hex(sb->buf, sha1)) {
> +	while (&sb.len && sb.buf[sb.len - 1] == '\n')
> +		strbuf_setlen(&sb, sb.len - 1);
> +	if (!sb.len)
> +		goto got_nothing;
> +	if (!prefixcmp(sb.buf, "refs/heads/"))
> +		strbuf_remove(&sb,0, strlen("refs/heads/"));
> +	else if (!prefixcmp(sb.buf, "refs/"))
> +		;
> +	else if (!get_sha1_hex(sb.buf, sha1)) {
>  		const char *abbrev;
>  		abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
> -		strbuf_reset(sb);
> -		strbuf_addstr(sb, abbrev);
> -		*branch = sb->buf;
> -	} else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
> -		;
> +		strbuf_reset(&sb);
> +		strbuf_addstr(&sb, abbrev);
> +	} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
> +		goto got_nothing;
>  	else			/* bisect */
> -		*branch = sb->buf;
> +		;
> +	return strbuf_detach(&sb, NULL);
> +
> +got_nothing:
> +	strbuf_release(&sb);
> +	return NULL;
>  }
>  
>  static void wt_status_print_state(struct wt_status *s)
>  {
>  	const char *state_color = color(WT_STATUS_HEADER, s);
> -	struct strbuf branch = STRBUF_INIT;
> -	struct strbuf onto = STRBUF_INIT;
>  	struct wt_status_state state;
>  	struct stat st;
>  
> @@ -1018,27 +1018,22 @@ static void wt_status_print_state(struct wt_status *s)
>  				state.am_empty_patch = 1;
>  		} else {
>  			state.rebase_in_progress = 1;
> -			read_and_strip_branch(&branch, &state.branch,
> -					      "rebase-apply/head-name");
> -			read_and_strip_branch(&onto, &state.onto,
> -					      "rebase-apply/onto");
> +			state.branch = read_and_strip_branch("rebase-apply/head-name");
> +			state.onto = read_and_strip_branch("rebase-apply/onto");
>  		}
>  	} else if (!stat(git_path("rebase-merge"), &st)) {
>  		if (!stat(git_path("rebase-merge/interactive"), &st))
>  			state.rebase_interactive_in_progress = 1;
>  		else
>  			state.rebase_in_progress = 1;
> -		read_and_strip_branch(&branch, &state.branch,
> -				      "rebase-merge/head-name");
> -		read_and_strip_branch(&onto, &state.onto,
> -				      "rebase-merge/onto");
> +		state.branch = read_and_strip_branch("rebase-merge/head-name");
> +		state.onto = read_and_strip_branch("rebase-merge/onto");
>  	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
>  		state.cherry_pick_in_progress = 1;
>  	}
>  	if (!stat(git_path("BISECT_LOG"), &st)) {
>  		state.bisect_in_progress = 1;
> -		read_and_strip_branch(&branch, &state.branch,
> -				      "BISECT_START");
> +		state.branch = read_and_strip_branch("BISECT_START");
>  	}
>  
>  	if (state.merge_in_progress)
> @@ -1051,8 +1046,8 @@ static void wt_status_print_state(struct wt_status *s)
>  		show_cherry_pick_in_progress(s, &state, state_color);
>  	if (state.bisect_in_progress)
>  		show_bisect_in_progress(s, &state, state_color);
> -	strbuf_release(&branch);
> -	strbuf_release(&onto);
> +	free(state.branch);
> +	free(state.onto);
>  }
>  
>  void wt_status_print(struct wt_status *s)
> diff --git a/wt-status.h b/wt-status.h
> index 81e1dcf..b8c3512 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -79,8 +79,8 @@ struct wt_status_state {
>  	int rebase_interactive_in_progress;
>  	int cherry_pick_in_progress;
>  	int bisect_in_progress;
> -	const char *branch;
> -	const char *onto;
> +	char *branch;
> +	char *onto;
>  };
>  
>  void wt_status_prepare(struct wt_status *s);

[PATCH v3+ 1/5] wt-status: move strbuf into read_and_strip_branch()

[Nguyễn Thái Ngọc Duy]

The strbufs are placed outside read_and_strip_branch as a premature
optimization: when it reads "refs/heads/foo" to strbuf and wants to
return just "foo", it could do so without memory movement. In return
the caller must not use the returned pointer after releasing strbufs,
which own the buffers that contain the returned strings. It's a clumsy
design.

By moving strbufs into read_and_strip_branch(), the returned pointer
always points to a malloc'd buffer or NULL. The pointer can be passed
around and freed after use.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Fixed the commit message. No code change.

 wt-status.c | 65 ++++++++++++++++++++++++++++---------------------------------
 wt-status.h |  4 ++--
 2 files changed, 32 insertions(+), 37 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index ef405d0..6cac27b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -969,41 +969,41 @@ static void show_bisect_in_progress(struct wt_status *s,
 /*
  * Extract branch information from rebase/bisect
  */
-static void read_and_strip_branch(struct strbuf *sb,
-				  const char **branch,
-				  const char *path)
+static char *read_and_strip_branch(const char *path)
 {
+	struct strbuf sb = STRBUF_INIT;
 	unsigned char sha1[20];
 
-	strbuf_reset(sb);
-	if (strbuf_read_file(sb, git_path("%s", path), 0) <= 0)
-		return;
+	if (strbuf_read_file(&sb, git_path("%s", path), 0) <= 0)
+		goto got_nothing;
 
-	while (sb->len && sb->buf[sb->len - 1] == '\n')
-		strbuf_setlen(sb, sb->len - 1);
-	if (!sb->len)
-		return;
-	if (!prefixcmp(sb->buf, "refs/heads/"))
-		*branch = sb->buf + strlen("refs/heads/");
-	else if (!prefixcmp(sb->buf, "refs/"))
-		*branch = sb->buf;
-	else if (!get_sha1_hex(sb->buf, sha1)) {
+	while (&sb.len && sb.buf[sb.len - 1] == '\n')
+		strbuf_setlen(&sb, sb.len - 1);
+	if (!sb.len)
+		goto got_nothing;
+	if (!prefixcmp(sb.buf, "refs/heads/"))
+		strbuf_remove(&sb,0, strlen("refs/heads/"));
+	else if (!prefixcmp(sb.buf, "refs/"))
+		;
+	else if (!get_sha1_hex(sb.buf, sha1)) {
 		const char *abbrev;
 		abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
-		strbuf_reset(sb);
-		strbuf_addstr(sb, abbrev);
-		*branch = sb->buf;
-	} else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
-		;
+		strbuf_reset(&sb);
+		strbuf_addstr(&sb, abbrev);
+	} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
+		goto got_nothing;
 	else			/* bisect */
-		*branch = sb->buf;
+		;
+	return strbuf_detach(&sb, NULL);
+
+got_nothing:
+	strbuf_release(&sb);
+	return NULL;
 }
 
 static void wt_status_print_state(struct wt_status *s)
 {
 	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct strbuf branch = STRBUF_INIT;
-	struct strbuf onto = STRBUF_INIT;
 	struct wt_status_state state;
 	struct stat st;
 
@@ -1018,27 +1018,22 @@ static void wt_status_print_state(struct wt_status *s)
 				state.am_empty_patch = 1;
 		} else {
 			state.rebase_in_progress = 1;
-			read_and_strip_branch(&branch, &state.branch,
-					      "rebase-apply/head-name");
-			read_and_strip_branch(&onto, &state.onto,
-					      "rebase-apply/onto");
+			state.branch = read_and_strip_branch("rebase-apply/head-name");
+			state.onto = read_and_strip_branch("rebase-apply/onto");
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
 			state.rebase_interactive_in_progress = 1;
 		else
 			state.rebase_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
-				      "rebase-merge/head-name");
-		read_and_strip_branch(&onto, &state.onto,
-				      "rebase-merge/onto");
+		state.branch = read_and_strip_branch("rebase-merge/head-name");
+		state.onto = read_and_strip_branch("rebase-merge/onto");
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
 		state.cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st)) {
 		state.bisect_in_progress = 1;
-		read_and_strip_branch(&branch, &state.branch,
-				      "BISECT_START");
+		state.branch = read_and_strip_branch("BISECT_START");
 	}
 
 	if (state.merge_in_progress)
@@ -1051,8 +1046,8 @@ static void wt_status_print_state(struct wt_status *s)
 		show_cherry_pick_in_progress(s, &state, state_color);
 	if (state.bisect_in_progress)
 		show_bisect_in_progress(s, &state, state_color);
-	strbuf_release(&branch);
-	strbuf_release(&onto);
+	free(state.branch);
+	free(state.onto);
 }
 
 void wt_status_print(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 81e1dcf..b8c3512 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,8 +79,8 @@ struct wt_status_state {
 	int rebase_interactive_in_progress;
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
-	const char *branch;
-	const char *onto;
+	char *branch;
+	char *onto;
 };
 
 void wt_status_prepare(struct wt_status *s);
-- 
1.8.2.83.gc99314b

[PATCH v3 2/5] wt-status: split wt_status_state parsing function out

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 41 +++++++++++++++++++++++------------------
 wt-status.h |  1 +
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 6cac27b..0d8989f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1001,40 +1001,45 @@ got_nothing:
 	return NULL;
 }
 
-static void wt_status_print_state(struct wt_status *s)
+void wt_status_get_state(struct wt_status_state *state)
 {
-	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
 	struct stat st;
 
-	memset(&state, 0, sizeof(state));
-
 	if (!stat(git_path("MERGE_HEAD"), &st)) {
-		state.merge_in_progress = 1;
+		state->merge_in_progress = 1;
 	} else if (!stat(git_path("rebase-apply"), &st)) {
 		if (!stat(git_path("rebase-apply/applying"), &st)) {
-			state.am_in_progress = 1;
+			state->am_in_progress = 1;
 			if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
-				state.am_empty_patch = 1;
+				state->am_empty_patch = 1;
 		} else {
-			state.rebase_in_progress = 1;
-			state.branch = read_and_strip_branch("rebase-apply/head-name");
-			state.onto = read_and_strip_branch("rebase-apply/onto");
+			state->rebase_in_progress = 1;
+			state->branch = read_and_strip_branch("rebase-apply/head-name");
+			state->onto = read_and_strip_branch("rebase-apply/onto");
 		}
 	} else if (!stat(git_path("rebase-merge"), &st)) {
 		if (!stat(git_path("rebase-merge/interactive"), &st))
-			state.rebase_interactive_in_progress = 1;
+			state->rebase_interactive_in_progress = 1;
 		else
-			state.rebase_in_progress = 1;
-		state.branch = read_and_strip_branch("rebase-merge/head-name");
-		state.onto = read_and_strip_branch("rebase-merge/onto");
+			state->rebase_in_progress = 1;
+		state->branch = read_and_strip_branch("rebase-merge/head-name");
+		state->onto = read_and_strip_branch("rebase-merge/onto");
 	} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
-		state.cherry_pick_in_progress = 1;
+		state->cherry_pick_in_progress = 1;
 	}
 	if (!stat(git_path("BISECT_LOG"), &st)) {
-		state.bisect_in_progress = 1;
-		state.branch = read_and_strip_branch("BISECT_START");
+		state->bisect_in_progress = 1;
+		state->branch = read_and_strip_branch("BISECT_START");
 	}
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+	const char *state_color = color(WT_STATUS_HEADER, s);
+	struct wt_status_state state;
+
+	memset(&state, 0, sizeof(state));
+	wt_status_get_state(&state);
 
 	if (state.merge_in_progress)
 		show_merge_in_progress(s, &state, state_color);
diff --git a/wt-status.h b/wt-status.h
index b8c3512..5ddcbf6 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -86,6 +86,7 @@ struct wt_status_state {
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
+void wt_status_get_state(struct wt_status_state *state);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

[PATCH v3 3/5] wt-status: move wt_status_get_state() out to wt_status_print()

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 0d8989f..17690d8 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1033,32 +1033,30 @@ void wt_status_get_state(struct wt_status_state *state)
 	}
 }
 
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_print_state(struct wt_status *s,
+				  struct wt_status_state *state)
 {
 	const char *state_color = color(WT_STATUS_HEADER, s);
-	struct wt_status_state state;
-
-	memset(&state, 0, sizeof(state));
-	wt_status_get_state(&state);
-
-	if (state.merge_in_progress)
-		show_merge_in_progress(s, &state, state_color);
-	else if (state.am_in_progress)
-		show_am_in_progress(s, &state, state_color);
-	else if (state.rebase_in_progress || state.rebase_interactive_in_progress)
-		show_rebase_in_progress(s, &state, state_color);
-	else if (state.cherry_pick_in_progress)
-		show_cherry_pick_in_progress(s, &state, state_color);
-	if (state.bisect_in_progress)
-		show_bisect_in_progress(s, &state, state_color);
-	free(state.branch);
-	free(state.onto);
+	if (state->merge_in_progress)
+		show_merge_in_progress(s, state, state_color);
+	else if (state->am_in_progress)
+		show_am_in_progress(s, state, state_color);
+	else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
+		show_rebase_in_progress(s, state, state_color);
+	else if (state->cherry_pick_in_progress)
+		show_cherry_pick_in_progress(s, state, state_color);
+	if (state->bisect_in_progress)
+		show_bisect_in_progress(s, state, state_color);
 }
 
 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);
+	struct wt_status_state state;
+
+	memset(&state, 0, sizeof(state));
+	wt_status_get_state(&state);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1077,7 +1075,10 @@ void wt_status_print(struct wt_status *s)
 			wt_status_print_tracking(s);
 	}
 
-	wt_status_print_state(s);
+	wt_status_print_state(s, &state);
+	free(state.branch);
+	free(state.onto);
+
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial commit"));
-- 
1.8.1.2.536.gf441e6d

[PATCH v3 4/5] status: show more info than "currently not on any branch"

[Nguyễn Thái Ngọc Duy]

When a remote ref or a tag is checked out, HEAD is automatically
detached. There is no user-friendly way to find out what ref is
checked out in this case. This patch digs in reflog for this
information and shows "HEAD detached from origin/master" or "HEAD
detached at v1.8.0" instead of "currently not on any branch".

When it cannot figure out the original ref, it shows an abbreviated
SHA-1. "Currently not on any branch" would never display (unless
reflog is pruned to near empty that the last checkout entry is lost).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t7406-submodule-update.sh |  6 ++-
 t/t7512-status-help.sh      | 52 ++++++++++++++++----------
 wt-status.c                 | 89 +++++++++++++++++++++++++++++++++++++++++++--
 wt-status.h                 |  4 +-
 4 files changed, 125 insertions(+), 26 deletions(-)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index ea61761..a4ffea0 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -665,8 +665,10 @@ test_expect_success 'submodule add properly re-creates deeper level submodules'
 
 test_expect_success 'submodule update properly revives a moved submodule' '
 	(cd super &&
+	 H=$(git rev-parse --short HEAD) &&
 	 git commit -am "pre move" &&
-	 git status >expect&&
+	 H2=$(git rev-parse --short HEAD) &&
+	 git status | sed "s/$H/XXX/" >expect &&
 	 H=$(cd submodule2; git rev-parse HEAD) &&
 	 git rm --cached submodule2 &&
 	 rm -rf submodule2 &&
@@ -675,7 +677,7 @@ test_expect_success 'submodule update properly revives a moved submodule' '
 	 git config -f .gitmodules submodule.submodule2.path "moved/sub module"
 	 git commit -am "post move" &&
 	 git submodule update &&
-	 git status >actual &&
+	 git status | sed "s/$H2/XXX/" >actual &&
 	 test_cmp expect actual
 	)
 '
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index d2da89a..da22088 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -76,7 +76,7 @@ test_expect_success 'status when rebase in progress before resolving conflicts'
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -103,7 +103,7 @@ test_expect_success 'status when rebase in progress before rebase --continue' '
 	echo three >main.txt &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -135,7 +135,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' '
 	ONTO=$(git rev-parse --short rebase_i_conflicts) &&
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -161,7 +161,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' '
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -187,9 +187,10 @@ test_expect_success 'status when rebasing -i in edit mode' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~2) &&
+	TGT=$(git rev-parse --short two_rebase_i) &&
 	git rebase -i HEAD~2 &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -214,8 +215,9 @@ test_expect_success 'status when splitting a commit' '
 	ONTO=$(git rev-parse --short HEAD~3) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
+	TGT=$(git rev-parse --short HEAD) &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $TGT
 	# You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -243,10 +245,11 @@ test_expect_success 'status after editing the last commit with --amend during a
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	TGT=$(git rev-parse --short three_amend) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -276,7 +279,7 @@ test_expect_success 'status: (continue first edit) second edit' '
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -298,7 +301,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -325,7 +328,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -347,7 +350,7 @@ test_expect_success 'status: (amend first edit) second edit' '
 	git commit --amend -m "a" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -370,7 +373,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -398,7 +401,7 @@ test_expect_success 'status: (amend first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "d" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -422,7 +425,7 @@ test_expect_success 'status: (split first edit) second edit' '
 	git commit -m "e" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -447,7 +450,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -477,7 +480,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "h" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -572,8 +575,9 @@ test_expect_success 'status when bisecting' '
 	git bisect start &&
 	git bisect bad &&
 	git bisect good one_bisect &&
-	cat >expected <<-\EOF &&
-	# Not currently on any branch.
+	TGT=$(git rev-parse --short two_bisect) &&
+	cat >expected <<-EOF &&
+	# HEAD detached at $TGT
 	# You are currently bisecting branch '\''bisect'\''.
 	#   (use "git bisect reset" to get back to the original branch)
 	#
@@ -596,7 +600,7 @@ test_expect_success 'status when rebase conflicts with statushints disabled' '
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
 	#
 	# Unmerged paths:
@@ -662,5 +666,15 @@ test_expect_success 'status when cherry-picking after resolving conflicts' '
 	test_i18ncmp expected actual
 '
 
+test_expect_success 'status showing detached from a tag' '
+	test_commit atag tagging &&
+	git checkout atag &&
+	cat >expected <<-\EOF
+	# HEAD detached at atag
+	nothing to commit (use -u to show untracked files)
+	EOF
+	git status --untracked-files=no >actual &&
+	test_i18ncmp expected actual
+'
 
 test_done
diff --git a/wt-status.c b/wt-status.c
index 17690d8..8c6e0c8 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1001,7 +1001,73 @@ got_nothing:
 	return NULL;
 }
 
-void wt_status_get_state(struct wt_status_state *state)
+struct grab_1st_switch_cbdata {
+	int found;
+	struct strbuf buf;
+	unsigned char nsha1[20];
+};
+
+static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+			   const char *email, unsigned long timestamp, int tz,
+			   const char *message, void *cb_data)
+{
+	struct grab_1st_switch_cbdata *cb = cb_data;
+	const char *target = NULL, *end;
+
+	if (prefixcmp(message, "checkout: moving from "))
+		return 0;
+	message += strlen("checkout: moving from ");
+	target = strstr(message, " to ");
+	if (!target)
+		return 0;
+	target += strlen(" to ");
+	strbuf_reset(&cb->buf);
+	hashcpy(cb->nsha1, nsha1);
+	for (end = target; *end && *end != '\n'; end++)
+		;
+	strbuf_add(&cb->buf, target, end - target);
+	cb->found = 1;
+	return 1;
+}
+
+static void wt_status_get_detached_from(struct wt_status_state *state)
+{
+	struct grab_1st_switch_cbdata cb;
+	struct commit *commit;
+	unsigned char sha1[20];
+	char *ref = NULL;
+
+	strbuf_init(&cb.buf, 0);
+	if (for_each_reflog_ent_reverse("HEAD", grab_1st_switch, &cb) <= 0) {
+		strbuf_release(&cb.buf);
+		return;
+	}
+
+	if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
+	    /* sha1 is a commit? match without further lookup */
+	    (!hashcmp(cb.nsha1, sha1) ||
+	     /* perhaps sha1 is a tag, try to dereference to a commit */
+	     ((commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
+	      !hashcmp(cb.nsha1, commit->object.sha1)))) {
+		int ofs;
+		if (!prefixcmp(ref, "refs/tags/"))
+			ofs = strlen("refs/tags/");
+		else if (!prefixcmp(ref, "refs/remotes/"))
+			ofs = strlen("refs/remotes/");
+		else
+			ofs = 0;
+		state->detached_from = xstrdup(ref + ofs);
+	} else
+		state->detached_from =
+			xstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));
+	hashcpy(state->detached_sha1, cb.nsha1);
+
+	free(ref);
+	strbuf_release(&cb.buf);
+}
+
+void wt_status_get_state(struct wt_status_state *state,
+			 int get_detached_from)
 {
 	struct stat st;
 
@@ -1031,6 +1097,9 @@ void wt_status_get_state(struct wt_status_state *state)
 		state->bisect_in_progress = 1;
 		state->branch = read_and_strip_branch("BISECT_START");
 	}
+
+	if (get_detached_from)
+		wt_status_get_detached_from(state);
 }
 
 static void wt_status_print_state(struct wt_status *s,
@@ -1056,7 +1125,8 @@ void wt_status_print(struct wt_status *s)
 	struct wt_status_state state;
 
 	memset(&state, 0, sizeof(state));
-	wt_status_get_state(&state);
+	wt_status_get_state(&state,
+			    s->branch && !strcmp(s->branch, "HEAD"));
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1064,9 +1134,19 @@ void wt_status_print(struct wt_status *s)
 		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.");
+			if (state.detached_from) {
+				unsigned char sha1[20];
+				branch_name = state.detached_from;
+				if (!get_sha1("HEAD", sha1) &&
+				    !hashcmp(sha1, state.detached_sha1))
+					on_what = _("HEAD detached at ");
+				else
+					on_what = _("HEAD detached from ");
+			} else {
+				branch_name = "";
+				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);
@@ -1078,6 +1158,7 @@ void wt_status_print(struct wt_status *s)
 	wt_status_print_state(s, &state);
 	free(state.branch);
 	free(state.onto);
+	free(state.detached_from);
 
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
diff --git a/wt-status.h b/wt-status.h
index 5ddcbf6..5cb7df9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -81,12 +81,14 @@ struct wt_status_state {
 	int bisect_in_progress;
 	char *branch;
 	char *onto;
+	char *detached_from;
+	unsigned char detached_sha1[20];
 };
 
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
-void wt_status_get_state(struct wt_status_state *state);
+void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

Re: [PATCH v3 4/5] status: show more info than "currently not on any branch"

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> When a remote ref or a tag is checked out, HEAD is automatically
> detached. There is no user-friendly way to find out what ref is
> checked out in this case. This patch digs in reflog for this
> information and shows "HEAD detached from origin/master" or "HEAD
> detached at v1.8.0" instead of "currently not on any branch".
>
> When it cannot figure out the original ref, it shows an abbreviated
> SHA-1. "Currently not on any branch" would never display (unless
> reflog is pruned to near empty that the last checkout entry is lost).
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

It does look like using the for_each_in_reverse() does make the
caller less error prone (by the way, I'd love to see people review,
poke holes and plug them in the implementation of reading the reflog
file in reverse; I am not sure if it covered all the corner cases
right).

Thanks.

[PATCH v3 5/5] branch: show more information when HEAD is detached

[Nguyễn Thái Ngọc Duy]

This prints more helpful info when HEAD is detached: is it detached
because of bisect or rebase? What is the original branch name in those
cases? Is it detached because the user checks out a remote ref or a
tag (and which one)?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c            | 26 +++++++++++++++++++++++++-
 t/t3203-branch-output.sh    |  6 +++---
 t/t6030-bisect-porcelain.sh |  2 +-
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 00d17d2..2ab0e4f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -18,6 +18,7 @@
 #include "string-list.h"
 #include "column.h"
 #include "utf8.h"
+#include "wt-status.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [options] [-r | -a] [--merged | --no-merged]"),
@@ -550,6 +551,29 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description(void)
+{
+	struct strbuf desc = STRBUF_INIT;
+	struct wt_status_state state;
+	memset(&state, 0, sizeof(state));
+	wt_status_get_state(&state, 1);
+	if (state.rebase_in_progress ||
+	    state.rebase_interactive_in_progress)
+		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+			    state.branch);
+	else if (state.bisect_in_progress)
+		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
+			    state.branch);
+	else if (state.detached_from)
+		strbuf_addf(&desc, _("(detached from %s)"),
+			    state.detached_from);
+	else
+		strbuf_addstr(&desc, _("(no branch)"));
+	free(state.branch);
+	free(state.onto);
+	free(state.detached_from);
+	return strbuf_detach(&desc, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +581,7 @@ static void show_detached(struct ref_list *ref_list)
 
 	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
 		struct ref_item item;
-		item.name = xstrdup(_("(no branch)"));
+		item.name = get_head_description();
 		item.width = utf8_strwidth(item.name);
 		item.kind = REF_LOCAL_BRANCH;
 		item.dest = NULL;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 76fe7e0..ba4f98e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -94,13 +94,13 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
 '
 
-cat >expect <<'EOF'
-* (no branch)
+test_expect_success 'git branch shows detached HEAD properly' '
+	cat >expect <<EOF &&
+* (detached from $(git rev-parse --short HEAD^0))
   branch-one
   branch-two
   master
 EOF
-test_expect_success 'git branch shows detached HEAD properly' '
 	git checkout HEAD^0 &&
 	git branch >actual &&
 	test_i18ncmp expect actual
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 3e0e15f..9b6f0d0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	cp .git/BISECT_START saved &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	test_i18ngrep "* (no branch)" branch.output > /dev/null &&
+	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
 	test_cmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
-- 
1.8.1.2.536.gf441e6d

Re: [PATCH v3 0/5] nd/branch-show-rebase-bisect-state updates

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This round fixes the design issue of read_and_strip_branch(), makes
> use of jc/reflog-reverse-walk and performs cheaper sha-1 check on
> detached commits.

I was running "git bisect start pu jch" (jch is a branch I do not
push out everywhere that is somewhere in the middle of
"master..pu"), and then after bisection finished, saw this:

    $ git branch --with 66887099e096f0258a5ef3b1e7458748597bdffe
    * (no branch, bisecting jch)
      nd/magic-pathspecs
      pu

which is obviously bogus.  I only said "I know the tip of pu is bad
and the tip of jch is good".  If anything, I was bisecting the
breakage in 'pu', not in 'jch'.

I think the message should better say "before you started to bisect
you were on 'jch'" or something instead.

Re: [PATCH v3 0/5] nd/branch-show-rebase-bisect-state updates

[Duy Nguyen]

On Wed, Mar 20, 2013 at 1:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I was running "git bisect start pu jch" (jch is a branch I do not
> push out everywhere that is somewhere in the middle of
> "master..pu"), and then after bisection finished, saw this:
>
>     $ git branch --with 66887099e096f0258a5ef3b1e7458748597bdffe
>     * (no branch, bisecting jch)
>       nd/magic-pathspecs
>       pu
>
> which is obviously bogus.  I only said "I know the tip of pu is bad
> and the tip of jch is good".  If anything, I was bisecting the
> breakage in 'pu', not in 'jch'.
>
> I think the message should better say "before you started to bisect
> you were on 'jch'" or something instead.

How about

* (no branch, bisect started on jch)

then? I don't want to make it too long because it'll waste space in
column layout.
-- 
Duy

Re: [PATCH v3 0/5] nd/branch-show-rebase-bisect-state updates

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> I think the message should better say "before you started to bisect
>> you were on 'jch'" or something instead.
>
> How about
>
> * (no branch, bisect started on jch)
>
> then?

Sure.

What you are showing is used only to decide where "git bisect reset"
goes to, I think.

[PATCH] status, branch: fix the misleading "bisecting" message

[Nguyễn Thái Ngọc Duy]

The current message is "bisecting %s" (or "bisecting branch %s").
"%s" is the current branch when we started bisecting. Clarify that to
avoid confusion with good and bad refs passed to "bisect" command.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c            | 2 +-
 t/t6030-bisect-porcelain.sh | 2 +-
 t/t7512-status-help.sh      | 2 +-
 wt-status.c                 | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 99105f8..8f00203 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -562,7 +562,7 @@ static char *get_head_description(void)
 		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
 			    state.branch);
 	else if (state.bisect_in_progress)
-		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
+		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
 			    state.branch);
 	else if (state.detached_from)
 		strbuf_addf(&desc, _("(detached from %s)"),
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9b6f0d0..2fce99a 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	cp .git/BISECT_START saved &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
+	test_i18ngrep "* (no branch, bisect started on other)" branch.output > /dev/null &&
 	test_cmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 5adba4f..c35d01d 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -578,7 +578,7 @@ test_expect_success 'status when bisecting' '
 	TGT=$(git rev-parse --short two_bisect) &&
 	cat >expected <<-EOF &&
 	# HEAD detached at $TGT
-	# You are currently bisecting branch '\''bisect'\''.
+	# You are currently bisecting, started from branch '\''bisect'\''.
 	#   (use "git bisect reset" to get back to the original branch)
 	#
 	nothing to commit (use -u to show untracked files)
diff --git a/wt-status.c b/wt-status.c
index 32a51e1..cf3d81a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -953,7 +953,7 @@ static void show_bisect_in_progress(struct wt_status *s,
 {
 	if (state->branch)
 		status_printf_ln(s, color,
-				 _("You are currently bisecting branch '%s'."),
+				 _("You are currently bisecting, started from branch '%s'."),
 				 state->branch);
 	else
 		status_printf_ln(s, color,
-- 
1.8.2.83.gc99314b

Re: [PATCH] status, branch: fix the misleading "bisecting" message

[Junio C Hamano]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> The current message is "bisecting %s" (or "bisecting branch %s").
> "%s" is the current branch when we started bisecting. Clarify that to
> avoid confusion with good and bad refs passed to "bisect" command.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Sounds sensible.  Is this something we should squash in to
nd/branch-show-rebase-bisect-state topic, or just apply it to the
top (the latter is easier for me, but you may want to keep the
history free of "oops, that was a bad idea" changes).

Will queue on top, at least for now, as the topic is not in 'next'.

Thanks.

>  builtin/branch.c            | 2 +-
>  t/t6030-bisect-porcelain.sh | 2 +-
>  t/t7512-status-help.sh      | 2 +-
>  wt-status.c                 | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 99105f8..8f00203 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -562,7 +562,7 @@ static char *get_head_description(void)
>  		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
>  			    state.branch);
>  	else if (state.bisect_in_progress)
> -		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
> +		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
>  			    state.branch);
>  	else if (state.detached_from)
>  		strbuf_addf(&desc, _("(detached from %s)"),
> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
> index 9b6f0d0..2fce99a 100755
> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
>  	cp .git/BISECT_START saved &&
>  	test_must_fail git bisect start $HASH4 foo -- &&
>  	git branch > branch.output &&
> -	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
> +	test_i18ngrep "* (no branch, bisect started on other)" branch.output > /dev/null &&
>  	test_cmp saved .git/BISECT_START
>  '
>  test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
> diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
> index 5adba4f..c35d01d 100755
> --- a/t/t7512-status-help.sh
> +++ b/t/t7512-status-help.sh
> @@ -578,7 +578,7 @@ test_expect_success 'status when bisecting' '
>  	TGT=$(git rev-parse --short two_bisect) &&
>  	cat >expected <<-EOF &&
>  	# HEAD detached at $TGT
> -	# You are currently bisecting branch '\''bisect'\''.
> +	# You are currently bisecting, started from branch '\''bisect'\''.
>  	#   (use "git bisect reset" to get back to the original branch)
>  	#
>  	nothing to commit (use -u to show untracked files)
> diff --git a/wt-status.c b/wt-status.c
> index 32a51e1..cf3d81a 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -953,7 +953,7 @@ static void show_bisect_in_progress(struct wt_status *s,
>  {
>  	if (state->branch)
>  		status_printf_ln(s, color,
> -				 _("You are currently bisecting branch '%s'."),
> +				 _("You are currently bisecting, started from branch '%s'."),
>  				 state->branch);
>  	else
>  		status_printf_ln(s, color,