threads / patch / 32769

patchbranch: show (rebasing) or (bisecting) instead of (no branch) when possible

Subject: [PATCH] branch: show (rebasing) or (bisecting) instead of (no branch) when possible

## tl;dr

14 messages between Jan 29, 2013 and Feb 14, 2013. Diffs are folded; open one to read it.

replies: 13people: 4as markdown or json

Nguyễn Thái Ngọc Duy· Jan 29, 2013, 12:12 UTC · lore
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 In the spirit of status' in-progress info. I think showing this is
 more useful than "(no branch)". I tend to do "git br" more often than
 "git st" and this catches my eyes.
 builtin/branch.c            | 10 +++++++++-
 t/t6030-bisect-porcelain.sh |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)
Show changes to 2 files +10 −2

builtin/branch.c, t/t6030-bisect-porcelain.sh

diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..b0c5a20 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -557,7 +557,15 @@ 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)"));
+		struct stat st;
+		if ((!stat(git_path("rebase-apply"), &st) &&
+		     stat(git_path("rebase-apply/applying"), &st)) ||
+		    !stat(git_path("rebase-merge"), &st))
+			item.name = xstrdup(_("(rebasing)"));
+		else if (!stat(git_path("BISECT_LOG"), &st))
+			item.name = xstrdup(_("(bisecting)"));
+		else
+			item.name = xstrdup(_("(no branch)"));
 		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..bc21bc9 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 "* (bisecting)" 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.1.459.g5970e58
Jonathan Nieder· Jan 29, 2013, 19:13 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] branch: show (rebasing) or (bisecting) instead of (no branch) when possible

Nguyễn Thái Ngọc Duy wrote:
Show 5 quoted lines
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  In the spirit of status' in-progress info. I think showing this is
>  more useful than "(no branch)". I tend to do "git br" more often than
>  "git st" and this catches my eyes.

Very nice idea. This would also have been a nice way to avoid confusion when my officemate used bisect for the first time.

Any particular reason the above explanation is after the triple-dash instead of before it?

[...]
Show 11 quoted lines
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -557,7 +557,15 @@ 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)"));
> +		struct stat st;
> +		if ((!stat(git_path("rebase-apply"), &st) &&
> +		     stat(git_path("rebase-apply/applying"), &st)) ||
> +		    !stat(git_path("rebase-merge"), &st))

Here's a straight translation of contrib/completion/prompt.sh for comparison, skipping the cases that don't involve automatically detaching HEAD:

	if (!stat(git_path("rebase-merge"), &st) && S_ISDIR(st.st_mode))
		item.name = xstrdup(_("(rebasing)"));
	else if (!access(git_path("rebase-apply/rebasing"), F_OK))
		item.name = xstrdup(_("(rebasing)"));
	else if (!access(git_path("BISECT_LOG"), F_OK))
		item.name = xstrdup(_("(bisecting)"));
	else
		item.name = xstrdup(_("(no branch)"));
That would mean:
 * using access() instead of stat() to avoid unnecessary work
 * relying on rebase--am to write .git/rebase-apply/rebasing when
   appropriate instead of guessing
Not important, though. :)
Jonathan
Nguyễn Thái Ngọc Duy· Feb 3, 2013, 05:48 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

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?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
  - Incorporate Jonathan's version of checking
  - Show original branch name, e.g. "(rebasing foo)", when possible
 builtin/branch.c            | 40 +++++++++++++++++++++++++++++++++++++++-
 t/t6030-bisect-porcelain.sh |  2 +-
 2 files changed, 40 insertions(+), 2 deletions(-)
Show changes to 2 files +40 −2

builtin/branch.c, t/t6030-bisect-porcelain.sh

diff --git a/builtin/branch.c b/builtin/branch.c
index ea6498b..40f20ad 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -550,6 +550,44 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description()
+{
+	struct stat st;
+	struct strbuf sb = STRBUF_INIT;
+	struct strbuf result = STRBUF_INIT;
+	int bisect = 0;
+	int ret;
+	if (!stat(git_path("rebase-merge"), &st) && S_ISDIR(st.st_mode))
+		ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 0);
+	else if (!access(git_path("rebase-apply/rebasing"), F_OK))
+		ret = strbuf_read_file(&sb, git_path("rebase-apply/head-name"), 0);
+	else if (!access(git_path("BISECT_LOG"), F_OK)) {
+		ret = strbuf_read_file(&sb, git_path("BISECT_START"), 0);
+		bisect = 1;
+	} else
+		return xstrdup(_("(no branch)"));
+
+	if (ret <= 0)
+		return xstrdup(bisect ? _("(bisecting)") : _("_(rebasing)"));
+
+	while (sb.len && sb.buf[sb.len - 1] == '\n')
+		strbuf_setlen(&sb, sb.len - 1);
+
+	if (bisect) {
+		unsigned char sha1[20];
+		if (!get_sha1_hex(sb.buf, sha1))
+			strbuf_addstr(&result, _("(bisecting)"));
+		else
+			strbuf_addf(&result, _("(bisecting %s)"), sb.buf);
+	} else {
+		if (!prefixcmp(sb.buf, "refs/heads/"))
+			strbuf_addf(&result, _("(rebasing %s)"), sb.buf + 11);
+		else
+			strbuf_addstr(&result, _("(rebasing)"));
+	}
+	strbuf_release(&sb);
+	return strbuf_detach(&result, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +595,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..90968d5 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 "* (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.1.459.g5970e58
Matthieu Moy· Feb 3, 2013, 21:23 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 8 quoted lines
> --- 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 "* (bisecting other)" branch.output > /dev/null &&

I'd have spelled it (no branch, bisecting other) to make it clear that we're on detached HEAD, and avoid confusing old-timers. But maybe your version is enough, I'm not sure.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· Feb 3, 2013, 21:58 UTC · re: Matthieu Moy · lore

Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 14 quoted lines
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> --- 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 "* (bisecting other)" branch.output > /dev/null &&
>
> I'd have spelled it (no branch, bisecting other) to make it clear that
> we're on detached HEAD, and avoid confusing old-timers. But maybe your
> version is enough, I'm not sure.
Yeah, I do not think "bisecting other" alone makes much sense.

What does "other" refer to when you start your bisection at a detached head? I personally think "other" has _any_ value in that message, because "(no branch, bisecting)" gives the same amount of information, especially because "other" does not say which branch it refers to at all.

Junio C Hamano· Feb 3, 2013, 22:00 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

Junio C Hamano <gitster@pobox.com> writes:
> Yeah, I do not think "bisecting other" alone makes much sense.
>
> What does "other" refer to when you start your bisection at a
> detached head?  I personally think "other" has _any_ value in that
s/_any_/_no_/; obviously ;-)
> message, because "(no branch, bisecting)" gives the same amount of
> information, especially because "other" does not say which branch it
> refers to at all.
Matthieu Moy· Feb 4, 2013, 13:13 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>>
>>> --- 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 "* (bisecting other)" branch.output > /dev/null &&
[...]
> What does "other" refer to when you start your bisection at a
> detached head?

If I read correctly, the branch name is shown only when the bisection was not started on a detached HEAD:

+ if (!get_sha1_hex(sb.buf, sha1)) + strbuf_addstr(&result, _("(bisecting)")); + else + strbuf_addf(&result, _("(bisecting %s)"), sb.buf);

> I personally think "other" has _no_ value in that message, because
> "(no branch, bisecting)" gives the same amount of information,
> especially because "other" does not say which branch it refers to at
> all.

Just to be sure: "other" here is not a hardcoded message "bisecting other", but an instance of "bisecting <branch-name>" with the branch being called "other".

I think this is very valuable information, and most likely the information you'd be looking for when running "git branch". You're technically on detached HEAD, but morally on some branch, the HEAD being detached only because a bisect is in progress.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· Feb 4, 2013, 16:18 UTC · re: Matthieu Moy · lore

Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Just to be sure: "other" here is not a hardcoded message "bisecting
> other", but an instance of "bisecting <branch-name>" with the branch
> being called "other".
OK, then I do not have any objection.  Thanks.
Duy Nguyen· Feb 4, 2013, 07:14 UTC · re: Matthieu Moy · lore

Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

On Mon, Feb 4, 2013 at 4:23 AM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 14 quoted lines
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> --- 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 "* (bisecting other)" branch.output > /dev/null &&
>
> I'd have spelled it (no branch, bisecting other) to make it clear that
> we're on detached HEAD, and avoid confusing old-timers. But maybe your
> version is enough, I'm not sure.
If we want to make it clear, I think the standard "* (no branch)" should become
* HEAD (detached)
or non-detached case:
* HEAD -> foo
Then we could present rebase/bisect information as
* HEAD (detached, bisecting)
* HEAD (detached, rebasing)
* foo (rebasing)

I don't want to make this line too long because it would break (well, waste space in) column layout. So if we do this, no branch name added for rebase/bisect.

-- 
Duy
Duy Nguyen· Feb 4, 2013, 07:17 UTC · re: Duy Nguyen · lore

Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"

On Mon, Feb 4, 2013 at 2:14 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> * foo (rebasing)
Well, this one does not make sense (or causes more confusion).
-- 
Duy
Nguyễn Thái Ngọc Duy· Feb 8, 2013, 10:09 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v3] branch: show rebase/bisect info when possible instead of "(no branch)"

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?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Keep "no branch" in all cases. Just append "rebasing/bisecting [%s]"
 when applicable.
 builtin/branch.c            | 44 +++++++++++++++++++++++++++++++++++++++++++-
 t/t6030-bisect-porcelain.sh |  2 +-
 2 files changed, 44 insertions(+), 2 deletions(-)
Show changes to 2 files +44 −2

builtin/branch.c, t/t6030-bisect-porcelain.sh

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..26c0c3d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -550,6 +550,48 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description()
+{
+	struct stat st;
+	struct strbuf sb = STRBUF_INIT;
+	struct strbuf result = STRBUF_INIT;
+	int bisect = 0;
+	int ret;
+	if (!stat(git_path("rebase-merge"), &st) && S_ISDIR(st.st_mode))
+		ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 0);
+	else if (!access(git_path("rebase-apply/rebasing"), F_OK))
+		ret = strbuf_read_file(&sb, git_path("rebase-apply/head-name"), 0);
+	else if (!access(git_path("BISECT_LOG"), F_OK)) {
+		ret = strbuf_read_file(&sb, git_path("BISECT_START"), 0);
+		bisect = 1;
+	} else
+		return xstrdup(_("(no branch)"));
+
+	if (ret <= 0) {
+		if (bisect)
+			return xstrdup(_("(no branch, bisecting)"));
+		else
+			return xstrdup(_("_(no branch, rebasing)"));
+	}
+
+	while (sb.len && sb.buf[sb.len - 1] == '\n')
+		strbuf_setlen(&sb, sb.len - 1);
+
+	if (bisect) {
+		unsigned char sha1[20];
+		if (!get_sha1_hex(sb.buf, sha1))
+			strbuf_addstr(&result, _("(no branch, bisecting)"));
+		else
+			strbuf_addf(&result, _("(no branch, bisecting %s)"), sb.buf);
+	} else {
+		if (!prefixcmp(sb.buf, "refs/heads/"))
+			strbuf_addf(&result, _("(no branch, rebasing %s)"), sb.buf + 11);
+		else
+			strbuf_addstr(&result, _("(no branch, rebasing)"));
+	}
+	strbuf_release(&sb);
+	return strbuf_detach(&result, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +599,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
Junio C Hamano· Feb 8, 2013, 18:35 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v3] branch: show rebase/bisect info when possible instead of "(no branch)"

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 39 quoted lines
> 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?
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Keep "no branch" in all cases. Just append "rebasing/bisecting [%s]"
>  when applicable.
>
>  builtin/branch.c            | 44 +++++++++++++++++++++++++++++++++++++++++++-
>  t/t6030-bisect-porcelain.sh |  2 +-
>  2 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 6371bf9..26c0c3d 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -550,6 +550,48 @@ static int calc_maxwidth(struct ref_list *refs)
>  	return w;
>  }
>  
> +static char *get_head_description()
> +{
> +	struct stat st;
> +	struct strbuf sb = STRBUF_INIT;
> +	struct strbuf result = STRBUF_INIT;
> +	int bisect = 0;
> +	int ret;
> +	if (!stat(git_path("rebase-merge"), &st) && S_ISDIR(st.st_mode))
> +		ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 0);
> +	else if (!access(git_path("rebase-apply/rebasing"), F_OK))
> +		ret = strbuf_read_file(&sb, git_path("rebase-apply/head-name"), 0);
> +	else if (!access(git_path("BISECT_LOG"), F_OK)) {
> +		ret = strbuf_read_file(&sb, git_path("BISECT_START"), 0);
> +		bisect = 1;
> +	} else
> +		return xstrdup(_("(no branch)"));
> +
> +	if (ret <= 0) {
Doesn't the general "negative signals an error" apply here?

The end result may be the same, as the later part of this function that uses sb with len==0 ends up showing the same "bisecting" (or "rebasing") without any other information, but the logic to reach that outcome looks wrong.

Show 24 quoted lines
> +		if (bisect)
> +			return xstrdup(_("(no branch, bisecting)"));
> +		else
> +			return xstrdup(_("_(no branch, rebasing)"));
> +	}
> +
> +	while (sb.len && sb.buf[sb.len - 1] == '\n')
> +		strbuf_setlen(&sb, sb.len - 1);
> +
> +	if (bisect) {
> +		unsigned char sha1[20];
> +		if (!get_sha1_hex(sb.buf, sha1))
> +			strbuf_addstr(&result, _("(no branch, bisecting)"));
> +		else
> +			strbuf_addf(&result, _("(no branch, bisecting %s)"), sb.buf);
> +	} else {
> +		if (!prefixcmp(sb.buf, "refs/heads/"))
> +			strbuf_addf(&result, _("(no branch, rebasing %s)"), sb.buf + 11);
> +		else
> +			strbuf_addstr(&result, _("(no branch, rebasing)"));
> +	}
> +	strbuf_release(&sb);
> +	return strbuf_detach(&result, NULL);
> +}

We may want to refactor wt_status_print_state() and its callee a bit so that it and this part can share the logic without duplication and risk implementing subtly different decision. wt_status used to have clean separation between collection phase and presentation phase, but the wall between the phases seems deteriorated over time as more "in progress" crufts have been piled on top.

Such a refactoring may look larger than necessary, but on the other hand, I do not see this feature very useful if it can over time drift away from what we will see in "git status", so...

Show 25 quoted lines
>  
>  static void show_detached(struct ref_list *ref_list)
>  {
> @@ -557,7 +599,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' '
Duy Nguyen· Feb 14, 2013, 09:46 UTC · re: Junio C Hamano · lore

Re: [PATCH v3] branch: show rebase/bisect info when possible instead of "(no branch)"

On Sat, Feb 9, 2013 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> We may want to refactor wt_status_print_state() and its callee a bit
> so that it and this part can share the logic without duplication and
> risk implementing subtly different decision.  wt_status used to have
> clean separation between collection phase and presentation phase,
> but the wall between the phases seems deteriorated over time as more
> "in progress" crufts have been piled on top.
>
> Such a refactoring may look larger than necessary, but on the other
> hand, I do not see this feature very useful if it can over time
> drift away from what we will see in "git status", so...

OK. I'll wait until nd/status-show-in-progress is merged to master, then rework this patch on top.

-- 
Duy
Junio C Hamano· Feb 11, 2013, 19:13 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v3] branch: show rebase/bisect info when possible instead of "(no branch)"

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 9 quoted lines
> +static char *get_head_description()
> +{
> +	struct stat st;
> +	struct strbuf sb = STRBUF_INIT;
> +	struct strbuf result = STRBUF_INIT;
> +	int bisect = 0;
> +	int ret;
> +	if (!stat(git_path("rebase-merge"), &st) && S_ISDIR(st.st_mode))
> +		ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 0);

Hrmph. Why isn't this checking if the file exists and then read it, i.e.

	if (access(git_path("rebase-merge/head-name"), F_OK))
		ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 0);

It is not like you are creating this file and making sure leading directories exist, so the sequence looks a bit strange.

Show 5 quoted lines
> +	else if (!access(git_path("rebase-apply/rebasing"), F_OK))
> +		ret = strbuf_read_file(&sb, git_path("rebase-apply/head-name"), 0);
> +	else if (!access(git_path("BISECT_LOG"), F_OK)) {
> +		ret = strbuf_read_file(&sb, git_path("BISECT_START"), 0);
> +		bisect = 1;

And if the answer to the above question is "because if rebase-merge/ exists, with or without head-name, we know we are not bisecting", then that may suggest that the structure of if/elseif cascade is misdesigned. Shouldn't the "bisect" boolean be an enum "what are we doing" that is initialized to "I do not know" and each of these if/elseif cascade set the state to it when they know what we are doing, in order for this function to be longer-term maintainable?

← back to recent threads