Re: [PATCH RESEND 2/2] status: improve info for detached HEAD after clone

[Junio C Hamano <gitster@pobox.com>]

Roy Eldar <royeldar0@gmail.com> writes:

> diff --git a/wt-status.c b/wt-status.c
> index 3162241a57..f0a5fb578a 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1632,6 +1632,13 @@ static int grab_1st_switch(struct object_id *ooid UNUSED,
>  	struct grab_1st_switch_cbdata *cb = cb_data;
>  	const char *target = NULL, *end;
>  
> +	if (skip_prefix(message, "clone: from ", &message)) {
> +		oidcpy(&cb->noid, noid);
> +		strbuf_reset(&cb->buf);
> +		strbuf_add_unique_abbrev(&cb->buf, noid, DEFAULT_ABBREV);
> +		return 1;
> +	}
> +
>  	if (!skip_prefix(message, "checkout: moving from ", &message))
>  		return 0;
>  	target = strstr(message, " to ");

Two comments:

 - The original seems to duplicate the logic already in use for
   parsing @{-1} in object-name.c::grab_nth_branch_switch().

 - Adding new code here would mean that the result of parsing @{-1}
   and what wt_status_get_detached_from() will report becomes
   inconsistent, no?  After such a clone, "git checkout @{-1}" would
   say "there is no @{-1}" but "git status" would say it was
   detached from some hexadecimal object.

Thinking about the latter, I think it does not add much value to say
that we detached from something that is not a ref, so not adding
"clone: from " logic to grab_nth_branch_switch() is probably the
right thing to do anyway.

But then does it even make sense to have this new logic here?

Yes, the head may be detached at some object that is not a local or
remote branch.  But what is so bad about reporting the fact
faithfully, i.e., that we are not on any branch?  What commit object
we are at can be seen by "git show" or "git rev-parse HEAD" or any
other usual ways anyway, so...

I personally do not very much appreciate the extra info that is
given by saying "HEAD detached at X" and "HEAD detached from X",
compared to saying just "Not currently on any branch", especially
when these X are not concrete branch names or tag names but just
hexadecimal string that needs to be fed to "git describe" to be
turned into something that makes sense to humans, and that is
probably the reason why I am not a good judge about the change this
patch makes.  Others who like the "detached at/from X" may be better
judges to decide if this change makes sense.

Thanks.