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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 6, 2015, 20:23 UTC
Message-ID: <xmqqtwxxyiw4.fsf@gitster.dls.corp.google.com>
In-Reply-To: <0ece5e04a3fb38bdc1201036d65ed98199758ba5.1425653932.git.git@drmicha.warpmail.net>

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

Show 23 quoted lines

> "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.