[RFC/PATCH] branch: name detached HEAD analogous to status

[RFC/PATCH] branch: name detached HEAD analogous to status

[Michael J Gruber]

"git status" carefully names a detached HEAD "at" resp. "from" a rev or
ref depending on whether the detached HEAD has moved since. "git branch"
always uses "from", which can be confusing, because a status-aware user
would interpret this as moved detached HEAD.

Make "git branch" use the same logic and wording.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---

Notes:
    The wording is still different:
    
    HEAD detached at %s
    * (detached at %s)
    
    for status (line 1) resp. branch (line 2). Maybe it's worthwhile to use the
    exact same string so that l10n output is guaranteed to be the same? E.g.
    
    a)
    HEAD detached at %s
    * (HEAD detached at %s)
    
    or
    b)
    HEAD (detached at %s)
    * (detached at %s)
    
    In case b), "git status" strings would need to change, in case a)
    only "git branch" strings which change anyway by this patch.

 builtin/branch.c         | 13 ++++++++++---
 t/t3203-branch-output.sh | 39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index d8949cb..be391ee 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -589,9 +589,16 @@ static char *get_head_description(void)
 	else if (state.bisect_in_progress)
 		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
 			    state.branch);
-	else if (state.detached_from)
-		strbuf_addf(&desc, _("(detached from %s)"),
-			    state.detached_from);
+	else if (state.detached_from) {
+		unsigned char sha1[20];
+		if (!get_sha1("HEAD", sha1) &&
+		    !hashcmp(sha1, state.detached_sha1))
+			strbuf_addf(&desc, _("(detached at %s)"),
+				state.detached_from);
+		else
+			strbuf_addf(&desc, _("(detached from %s)"),
+				state.detached_from);
+	}
 	else
 		strbuf_addstr(&desc, _("(no branch)"));
 	free(state.branch);
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index ba4f98e..aaff885 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -96,7 +96,7 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 
 test_expect_success 'git branch shows detached HEAD properly' '
 	cat >expect <<EOF &&
-* (detached from $(git rev-parse --short HEAD^0))
+* (detached at $(git rev-parse --short HEAD^0))
   branch-one
   branch-two
   master
@@ -106,4 +106,41 @@ EOF
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'git branch shows detached HEAD properly after moving' '
+	cat >expect <<EOF &&
+* (detached from $(git rev-parse --short HEAD))
+  branch-one
+  branch-two
+  master
+EOF
+	git reset --hard HEAD^1 &&
+	git branch >actual &&
+	test_i18ncmp expect actual
+'
+
+test_expect_success 'git branch shows detached HEAD properly from tag' '
+	cat >expect <<EOF &&
+* (detached at fromtag)
+  branch-one
+  branch-two
+  master
+EOF
+	git tag fromtag master &&
+	git checkout fromtag &&
+	git branch >actual &&
+	test_i18ncmp expect actual
+'
+
+test_expect_success 'git branch shows detached HEAD properly after moving from tag' '
+	cat >expect <<EOF &&
+* (detached from fromtag)
+  branch-one
+  branch-two
+  master
+EOF
+	git reset --hard HEAD^1 &&
+	git branch >actual &&
+	test_i18ncmp expect actual
+'
+
 test_done
-- 
2.3.0.296.g32c87e1

Re: [RFC/PATCH] branch: name detached HEAD analogous to status

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> "git status" carefully names a detached HEAD "at" resp. "from" a rev or
> ref depending on whether the detached HEAD has moved since. "git branch"
> always uses "from", which can be confusing, because a status-aware user
> would interpret this as moved detached HEAD.
>
> Make "git branch" use the same logic and wording.

Yeah, otherwise the user would wonder why sometimes the object name
after that "from" matches "git rev-parse HEAD" and sometimes does
not.

In order to make sure that it will be easy for us to maintain that
these two commands will keep using the same logic and wording after
this "fix" is applied, should this patch do a bit more?  Or is it
worth doing that for such a small piece of code to be shared?

The following is a tangent and I do not think it is likely we would
do anything about it, but I wonder what value we give the end users
by showing the "from" information, both in "status" and "branch" in
the first place.  When I am on a detached HEAD, I'd be doing one of
these three things:

 (1) I am on some kind of sequencing machinery (e.g. "rebase -i",
     "cherry-pick A..B", or "bisect").  It does not matter to me at
     all if I am at the same commit at which I started the sequenced
     operations or the sequencing machinery has moved me one or more
     commits along its planned course of action, or where the
     original point the sequencing machinery detached the HEAD at.
     I suspect that I would not use "git status" or "git branch" in
     this mode anyway.

 (2) I am sight-seeing, starting with e.g. "git checkout v2.0.0",
     and moving around with "git checkout $some_other_commit".  I'd
     always see that I am "at" the commit I last checked out, so the
     distinctions would not be even shown to me.

 (3) I am experimenting to fix or enhance an existing thing that is
     meant to eventually hit a concrete branch, but I do not know if
     the experiment would pan out. "git checkout $topic~$n" would be
     to start from near the tip of that $topic ($n may often be 0
     but not always) and then I would "git commit" my experiments.
     When I assess my progress, I'd be interested in what I have
     that is not in $topic and vice versa since I started that
     experiment, so

     $ git log ...$topic
     $ git show-branch HEAD $topic

     would be a lot more useful than having to learn "where did I
     detach" from either "status" or "branch" and then do something
     about that the abbreviated object name (like feeding it to
     "describe" or "log").

Of course, the decision to make the point the HEAD was originally
detached at is not an issue this patch introduces, but it makes me
wonder if that existing "at vs from" logic is an overall win or a
loss.

Re: [RFC/PATCH] branch: name detached HEAD analogous to status

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 22.02.2015 20:21:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> "git status" carefully names a detached HEAD "at" resp. "from" a rev or
>> ref depending on whether the detached HEAD has moved since. "git branch"
>> always uses "from", which can be confusing, because a status-aware user
>> would interpret this as moved detached HEAD.
>>
>> Make "git branch" use the same logic and wording.
> 
> Yeah, otherwise the user would wonder why sometimes the object name
> after that "from" matches "git rev-parse HEAD" and sometimes does
> not.
> 
> In order to make sure that it will be easy for us to maintain that
> these two commands will keep using the same logic and wording after
> this "fix" is applied, should this patch do a bit more?  Or is it
> worth doing that for such a small piece of code to be shared?

Yes, I guess I meant RFD when I meant RFC. If that consistency is deemed
worthwhile it should at least be guaranteed by the tests, which the test
amendments somehow do, but better by a shared code in wt-status.c.

That could possibly be reused by the decorate code - which is how I came
about this: In order to decide about consistent HEAD decorations I
checked what we have and got confused by status vs. branch.

> The following is a tangent and I do not think it is likely we would
> do anything about it, but I wonder what value we give the end users
> by showing the "from" information, both in "status" and "branch" in
> the first place.  When I am on a detached HEAD, I'd be doing one of
> these three things:
> 
>  (1) I am on some kind of sequencing machinery (e.g. "rebase -i",
>      "cherry-pick A..B", or "bisect").  It does not matter to me at
>      all if I am at the same commit at which I started the sequenced
>      operations or the sequencing machinery has moved me one or more
>      commits along its planned course of action, or where the
>      original point the sequencing machinery detached the HEAD at.
>      I suspect that I would not use "git status" or "git branch" in
>      this mode anyway.
> 
>  (2) I am sight-seeing, starting with e.g. "git checkout v2.0.0",
>      and moving around with "git checkout $some_other_commit".  I'd
>      always see that I am "at" the commit I last checked out, so the
>      distinctions would not be even shown to me.
> 
>  (3) I am experimenting to fix or enhance an existing thing that is
>      meant to eventually hit a concrete branch, but I do not know if
>      the experiment would pan out. "git checkout $topic~$n" would be
>      to start from near the tip of that $topic ($n may often be 0
>      but not always) and then I would "git commit" my experiments.
>      When I assess my progress, I'd be interested in what I have
>      that is not in $topic and vice versa since I started that
>      experiment, so
> 
>      $ git log ...$topic
>      $ git show-branch HEAD $topic
> 
>      would be a lot more useful than having to learn "where did I
>      detach" from either "status" or "branch" and then do something
>      about that the abbreviated object name (like feeding it to
>      "describe" or "log").
> 
> Of course, the decision to make the point the HEAD was originally
> detached at is not an issue this patch introduces, but it makes me
> wonder if that existing "at vs from" logic is an overall win or a
> loss.

Not for you nor anyone who routinely detaches heads :)

Despite HEAD reflog and delayed pruning and all that, "detached HEAD" is
a state the average user may feel slightly uncomfortable with, and may
not even have gotten into on purpose. "git checkout tag" and "git
checkout remotebranch" are very easy ways to get there, even "git
checkout HEAD^1" and such when mistaking "checkout" for "reset".
Therefore, I think about that "at/from" as information (or rather: quick
guesstimate) on two things:

- How did I get there? (For this it might be better to say 'at/from
HEAD^1' which was sha1" rather than resolving that to a sha1 only. I
dunno. Detached heads move so easily...)

- Has something (that could get lost) happened since?

We take a quick and overly cautious approach to answering the 2nd
question, of course.

Maybe a "git head" command would be really a better place for all that
information:

git head
 "master" or "HEAD" (on branch resp. detached state)
git head -v
 "master at..." or "HEAD at ..., detached from/at..."
git head -l
 list of sha1s of childless prunable commits from HEAD's reflog
git head -d|--detach
 alias for "git checkout -detach HEAD"

(just brainstorming)

Michael

Re: [RFC/PATCH] branch: name detached HEAD analogous to status

[Marc Branchaud]

On 15-02-22 12:38 PM, Michael J Gruber wrote:
> "git status" carefully names a detached HEAD "at" resp. "from" a rev or
> ref depending on whether the detached HEAD has moved since. "git branch"
> always uses "from", which can be confusing, because a status-aware user
> would interpret this as moved detached HEAD.
> 
> Make "git branch" use the same logic and wording.

Except that the wording in "git branch" is more correct, especially if the
detached HEAD contains new commits.

In other words, "at" is only correct if the detached HEAD matches the ref.
If the HEAD has other commits, it is no longer "at" that ref but instead it
has grown "from" it.

But even if the detached HEAD matches the ref, saying it came "from" that ref
(with 0 extra commits) is still better than saying
detached-HEAD-with-extra-commits is "at" the ref.

		M.

Re: [RFC/PATCH] branch: name detached HEAD analogous to status

[Marc Branchaud]

On 15-02-22 02:21 PM, Junio C Hamano wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> "git status" carefully names a detached HEAD "at" resp. "from" a rev or
>> ref depending on whether the detached HEAD has moved since. "git branch"
>> always uses "from", which can be confusing, because a status-aware user
>> would interpret this as moved detached HEAD.
>>
>> Make "git branch" use the same logic and wording.
> 
> Yeah, otherwise the user would wonder why sometimes the object name
> after that "from" matches "git rev-parse HEAD" and sometimes does
> not.
> 
> In order to make sure that it will be easy for us to maintain that
> these two commands will keep using the same logic and wording after
> this "fix" is applied, should this patch do a bit more?  Or is it
> worth doing that for such a small piece of code to be shared?
> 
> The following is a tangent and I do not think it is likely we would
> do anything about it, but I wonder what value we give the end users
> by showing the "from" information, both in "status" and "branch" in
> the first place.  When I am on a detached HEAD, I'd be doing one of
> these three things:
> 
>  (1) I am on some kind of sequencing machinery (e.g. "rebase -i",
>      "cherry-pick A..B", or "bisect").  It does not matter to me at
>      all if I am at the same commit at which I started the sequenced
>      operations or the sequencing machinery has moved me one or more
>      commits along its planned course of action, or where the
>      original point the sequencing machinery detached the HEAD at.
>      I suspect that I would not use "git status" or "git branch" in
>      this mode anyway.
> 
>  (2) I am sight-seeing, starting with e.g. "git checkout v2.0.0",
>      and moving around with "git checkout $some_other_commit".  I'd
>      always see that I am "at" the commit I last checked out, so the
>      distinctions would not be even shown to me.
> 
>  (3) I am experimenting to fix or enhance an existing thing that is
>      meant to eventually hit a concrete branch, but I do not know if
>      the experiment would pan out. "git checkout $topic~$n" would be
>      to start from near the tip of that $topic ($n may often be 0
>      but not always) and then I would "git commit" my experiments.
>      When I assess my progress, I'd be interested in what I have
>      that is not in $topic and vice versa since I started that
>      experiment, so

I find it very useful, because I often work on HEADs detached from remote
branches ("git checkout origin/foo").  If I'm sight-seeing, I like the
reminder of which remote branch I checked out, especially because I often
have several working tress going at the same time.  I also often make trivial
changes, like typo fixes, on such detached HEADs, and here too I appreciate
the reminder of which remote branch I should push HEAD to.

		M.

Re: [RFC/PATCH] branch: name detached HEAD analogous to status

[Michael J Gruber]

Marc Branchaud venit, vidit, dixit 23.02.2015 16:12:
> On 15-02-22 12:38 PM, Michael J Gruber wrote:
>> "git status" carefully names a detached HEAD "at" resp. "from" a rev or
>> ref depending on whether the detached HEAD has moved since. "git branch"
>> always uses "from", which can be confusing, because a status-aware user
>> would interpret this as moved detached HEAD.
>>
>> Make "git branch" use the same logic and wording.
> 
> Except that the wording in "git branch" is more correct, especially if the
> detached HEAD contains new commits.
> 
> In other words, "at" is only correct if the detached HEAD matches the ref.
> If the HEAD has other commits, it is no longer "at" that ref but instead it
> has grown "from" it.

Sure, but that's exactly what git status does. Haven't you tried out?

And it's exactly what I suggest for git branch. It conveys more information.

> But even if the detached HEAD matches the ref, saying it came "from" that ref
> (with 0 extra commits) is still better than saying
> detached-HEAD-with-extra-commits is "at" the ref.

Why? Both are true. "at" conveys the additional information that HEAD is
still at the that rev.

Michael

Re: [RFC/PATCH] branch: name detached HEAD analogous to status

[Marc Branchaud]

On 15-02-23 11:24 AM, Michael J Gruber wrote:
> Marc Branchaud venit, vidit, dixit 23.02.2015 16:12:
>> On 15-02-22 12:38 PM, Michael J Gruber wrote:
>>> "git status" carefully names a detached HEAD "at" resp. "from" a rev or
>>> ref depending on whether the detached HEAD has moved since. "git branch"
>>> always uses "from", which can be confusing, because a status-aware user
>>> would interpret this as moved detached HEAD.
>>>
>>> Make "git branch" use the same logic and wording.
>>
>> Except that the wording in "git branch" is more correct, especially if the
>> detached HEAD contains new commits.
>>
>> In other words, "at" is only correct if the detached HEAD matches the ref.
>> If the HEAD has other commits, it is no longer "at" that ref but instead it
>> has grown "from" it.
> 
> Sure, but that's exactly what git status does. Haven't you tried out?
> 
> And it's exactly what I suggest for git branch. It conveys more information.

Oops, right.  Sorry, I got blinded by the various "detached at" examples in
your patch's notes.

		M.

[PATCHv2 0/2] branch output for detached HEAD

[Michael J Gruber]

So here's a little refactoring of wt-status, to help branch
use the same logic regarding from/at for a detached HEAD.

Michael J Gruber (2):
  wt-status: refactor detached HEAD analysis
  branch: name detached HEAD analogous to status

 builtin/branch.c         | 13 ++++++++++---
 t/t3203-branch-output.sh | 39 ++++++++++++++++++++++++++++++++++++++-
 wt-status.c              |  6 +++---
 wt-status.h              |  1 +
 4 files changed, 52 insertions(+), 7 deletions(-)

-- 
2.3.1.303.g5174db1

[PATCHv2 1/2] wt-status: refactor detached HEAD analysis

[Michael J Gruber]

wt_status_print() is the only caller of wt_status_get_detached_from().
The latter performs most of the analysis of a detached HEAD, including
finding state->detached_from; the caller checks whether the detached
HEAD is still at state->detached_from or has moved away.

Move that last bit of analysis to wt_status_get_detached_from(), too,
and store the boolean result in state->detached_at.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 wt-status.c | 6 +++---
 wt-status.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 29666d0..e7c1a4b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1222,6 +1222,8 @@ static void wt_status_get_detached_from(struct wt_status_state *state)
 		state->detached_from =
 			xstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));
 	hashcpy(state->detached_sha1, cb.nsha1);
+	state->detached_at = !get_sha1("HEAD", sha1) &&
+			     !hashcmp(sha1, state->detached_sha1);
 
 	free(ref);
 	strbuf_release(&cb.buf);
@@ -1310,10 +1312,8 @@ void wt_status_print(struct wt_status *s)
 				on_what = _("rebase in progress; onto ");
 				branch_name = state.onto;
 			} else if (state.detached_from) {
-				unsigned char sha1[20];
 				branch_name = state.detached_from;
-				if (!get_sha1("HEAD", sha1) &&
-				    !hashcmp(sha1, state.detached_sha1))
+				if (state.detached_at)
 					on_what = _("HEAD detached at ");
 				else
 					on_what = _("HEAD detached from ");
diff --git a/wt-status.h b/wt-status.h
index 283a9fe..e0a99f7 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -84,6 +84,7 @@ struct wt_status_state {
 	int cherry_pick_in_progress;
 	int bisect_in_progress;
 	int revert_in_progress;
+	int detached_at;
 	char *branch;
 	char *onto;
 	char *detached_from;
-- 
2.3.1.303.g5174db1

[PATCHv2 2/2] branch: name detached HEAD analogous to status

[Michael J Gruber]

"git status" carefully names a detached HEAD "at" resp. "from" a rev or
ref depending on whether the detached HEAD has moved since. "git branch"
always uses "from", which can be confusing, because a status-aware user
would interpret this as moved detached HEAD.

Make "git branch" use the same logic and wording.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---

Notes:
    v2 uses the info from refactored wt-status.
    
    In addition, it tries to make sure that branch and status use the same
    strings:
    
    HEAD detached at %s
    * (HEAD detached at %s)
    
    (status first line, branch second line)
    
    Unfortunately, status strings are broken into pieces, so this can
    be achieved by comments only.

 builtin/branch.c         | 13 ++++++++++---
 t/t3203-branch-output.sh | 39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index d8949cb..4c54240 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -589,9 +589,16 @@ static char *get_head_description(void)
 	else if (state.bisect_in_progress)
 		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
 			    state.branch);
-	else if (state.detached_from)
-		strbuf_addf(&desc, _("(detached from %s)"),
-			    state.detached_from);
+	else if (state.detached_from) {
+		/* TRANSLATORS: make sure these match _("HEAD detached at ")
+		   and _("HEAD detached from ") in wt-status.c */
+		if (state.detached_at)
+			strbuf_addf(&desc, _("(HEAD detached at %s)"),
+				state.detached_from);
+		else
+			strbuf_addf(&desc, _("(HEAD detached from %s)"),
+				state.detached_from);
+	}
 	else
 		strbuf_addstr(&desc, _("(no branch)"));
 	free(state.branch);
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index ba4f98e..f51d0f3 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -96,7 +96,7 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 
 test_expect_success 'git branch shows detached HEAD properly' '
 	cat >expect <<EOF &&
-* (detached from $(git rev-parse --short HEAD^0))
+* (HEAD detached at $(git rev-parse --short HEAD^0))
   branch-one
   branch-two
   master
@@ -106,4 +106,41 @@ EOF
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'git branch shows detached HEAD properly after moving' '
+	cat >expect <<EOF &&
+* (HEAD detached from $(git rev-parse --short HEAD))
+  branch-one
+  branch-two
+  master
+EOF
+	git reset --hard HEAD^1 &&
+	git branch >actual &&
+	test_i18ncmp expect actual
+'
+
+test_expect_success 'git branch shows detached HEAD properly from tag' '
+	cat >expect <<EOF &&
+* (HEAD detached at fromtag)
+  branch-one
+  branch-two
+  master
+EOF
+	git tag fromtag master &&
+	git checkout fromtag &&
+	git branch >actual &&
+	test_i18ncmp expect actual
+'
+
+test_expect_success 'git branch shows detached HEAD properly after moving from tag' '
+	cat >expect <<EOF &&
+* (HEAD detached from fromtag)
+  branch-one
+  branch-two
+  master
+EOF
+	git reset --hard HEAD^1 &&
+	git branch >actual &&
+	test_i18ncmp expect actual
+'
+
 test_done
-- 
2.3.1.303.g5174db1

Re: [PATCHv2 2/2] branch: name detached HEAD analogous to status

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> "git status" carefully names a detached HEAD "at" resp. "from" a rev or
> ref depending on whether the detached HEAD has moved since. "git branch"
> always uses "from", which can be confusing, because a status-aware user
> would interpret this as moved detached HEAD.
>
> Make "git branch" use the same logic and wording.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>
> Notes:
>     v2 uses the info from refactored wt-status.
>     
>     In addition, it tries to make sure that branch and status use the same
>     strings:
>     
>     HEAD detached at %s
>     * (HEAD detached at %s)
>     
>     (status first line, branch second line)
>     
>     Unfortunately, status strings are broken into pieces, so this can
>     be achieved by comments only.

It feels somewhat strange to have fields called detached-at and
detached-from in wt_status_state.

With this patch,

    * branch is either a string "HEAD" or the name of the branch we
      are on.

    * detached_from is the name of the branch HEAD detached from,
      or NULL if HEAD is on a branch.

    * detached_sha1[] is the object name of the commit we last
      moved to with "checkout --detach".

    * detached_at is a boolean that says the tip of detached_from
      and detached_sha1[] is the same.

There aren't that many users of these fields, so if we were to
rename them and clarify what they mean in order to make the code
more readable, we may be able to do so without too much churn.

Random conflicting/incoherent thoughts include:

 - perhaps branch can be NULL when detached?

 - perhaps branch can always point at the branch or the current
   detached-from?
 
 - perhaps detached_from can be renamed to make it more clear that
   it is the name of a branch (detached_sha1[] is fine, because it
   is clear what it is by having "sha1" in its name)?

 - perhaps detached_from/detached_at can be made into a single
   string that is set up by wt-status to hold either "HEAD detached
   at %s" or "HEAD detached from %s" to be used by its callers?

but I cannot yet reach a coherent whole to be called a suggestion
for a better organization X-<.

Will replace what was queued with this version.

Thanks.