Re: [PATCH v3 2/2] Doc/check-ref-format: clarify information about @{-N} syntax

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

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

> When the N-th previous thing checked out syntax (@{-N}) is used
> with '--branch' option of check-ref-format the result might not
> always be a valid branch name (see NOTE below). This is because
> @{-N} is used to refer to the N-th last checked out "thing" which
> might be any commit (sometimes a branch). The documentation thus
> does a wrong thing by promoting it as the "previous branch syntax".
>
> So, correctly state @{-N} is the syntax for specifying "N-th last
> thing checked out" and also state that the result of using @{-N}
> might also result in a "commit hash".
>
> NOTE: Though a commit-hash is a "syntactically" valid branch name,
> it is generally not considered as one for the use cases of
> "git check-ref-format --branch". That's because a user who does
> "git check-ref-format --branch @{-$N}" would except the output
> to be a "existing" branch name apart from it being syntactically
> valid.

s/except/expect/ I suspect.  But I do not think this description is
correct.  "check-ref-format --branch @{-1}", when you come from the
detached HEAD state, happily report success so it does not hold that
it is "generally not considered".

Unless you are saying "check-ref-format --branch" is buggy, that is.
If so, we shouldn't be casting that buggy behaviour in stone by
documenting, but should be fixing it.

But because this patch is about documenting, the farthest you can go
is to say that "check-ref-format --branch only checks if the name is
syntactically valid, and if you came from a detached HEAD, or if you
came from a branch that you deleted since then, the command will say
'yes, that looks like a good branch name to use'.  That may not
match what you expect, but that is the way it is.  Get used to it
and that is why we document that behaviour here."

And the paragraph that leads to this NOTE and this NOTE itself are
both misleading from that point of view.  The result *is* always a
valid branch name, but it may not name a branch that currently
exists or ever existed.

Taking the above together, perhaps.

    When the N-th previous thing checked out syntax (@{-N}) is used
    with '--branch' option of check-ref-format the result may not be
    the name of a branch that currently exists or ever existed.
    This is because @{-N} is used to refer to the N-th last checked
    out "thing", which might be any commit (sometimes a branch) in
    the detached HEAD state. The documentation thus does a wrong
    thing by promoting it as the "previous branch syntax".

    State that @{-N} is the syntax for specifying "N-th last thing
    checked out" and also state that the result of using @{-N} might
    also result in an commit object name.

> diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
> index cf0a0b7df..5ddb562d0 100644
> --- a/Documentation/git-check-ref-format.txt
> +++ b/Documentation/git-check-ref-format.txt
> @@ -78,17 +78,20 @@ reference name expressions (see linkgit:gitrevisions[7]):
>  . at-open-brace `@{` is used as a notation to access a reflog entry.
>  
>  With the `--branch` option, the command takes a name and checks if
> -it can be used as a valid branch name (e.g. when creating a new
> -branch).  The rule `git check-ref-format --branch $name` implements
> +it can be used as a valid branch name e.g. when creating a new branch
> +(except for one exception related to the previous checkout syntax
> +noted below). The rule `git check-ref-format --branch $name` implements

And "except for" is also wrong here.  40-hex still *IS* a valid
branch name; it is just it may not be what we expect.  So perhaps
rephrase it to something like "(but be cautious when using the
previous checkout syntax that may refer to a detached HEAD state)".

>  may be stricter than what `git check-ref-format refs/heads/$name`
>  says (e.g. a dash may appear at the beginning of a ref component,
>  but it is explicitly forbidden at the beginning of a branch name).
>  When run with `--branch` option in a repository, the input is first
> -expanded for the ``previous branch syntax''
> -`@{-n}`.  For example, `@{-1}` is a way to refer the last branch you
> -were on.  This option should be used by porcelains to accept this
> -syntax anywhere a branch name is expected, so they can act as if you
> -typed the branch name.
> +expanded for the ``previous checkout syntax''
> +`@{-n}`.  For example, `@{-1}` is a way to refer the last thing that
> +was checkout using "git checkout" operation. This option should be

s/was checkout/was checked out/;

> +used by porcelains to accept this syntax anywhere a branch name is
> +expected, so they can act as if you typed the branch name. As an
> +exception note that, the ``previous checkout operation'' might result
> +in a commit hash when the N-th last thing checked out was not a branch.

s/a commit hash/a commit object name/;