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

On Sat, 2017-12-02 at 17:52 -0800, Junio C Hamano wrote:

Kaartic Sivaraam [off-list ref] writes:

quoted

quoted

I have a mild suspicion that "git checkout -B @{-1}" would want to
error out instead of creating a valid new branch whose name is
40-hex that happen to be the name of the commit object you were
detached at previously.

I thought this the other way round. Rather than letting the callers
error out when @{-N} didn't expand to a branch name, I thought we
should not be expanding @{-N} syntax for "check-ref-format --branch" at
all to make a "stronger guarantee" that the result is "always" a valid
branch name. Then I thought it might be too restrictive and didn't
mention it. So, I dunno.

OK. Seems I was out of mind in the above reply. I have answered the
question about whether "branch=$(git check-ref-format @{-1}) && git
checkout -B $branch" should fail or not. Sorry, for the confusion in
case you mind it.

quoted

quoted

I am not sure if "check-ref-format --branch" should the same; it is
more about the syntax and the 40-hex _is_ valid there, so...

I'm not sure what you were trying to say here, sorry.

The "am not sure if" comes from this question: should these two be
equivalent?

    $ git check-ref-format --branch @{-1}
    $ git check-ref-format --branch $(git rev-parse --verify @{-1})

I could see how,

	$ git rev-parse --verify @{-1}
	$ git rev-parse --verify $(git check-ref-format --branch @{-1})

could be equivalent. I'm not sure how the previous two might be
equivalent except in the case when the previous thing checked out was
not a branch.

1. "git check-ref-format --branch @{-1}" returns the previous thing
checked out which might be a branch name or a commit hash

2. "git check-ref-format --branch $(git rev-parse --verify @{-1})"
returns the commit hash of the previous thing (the hash of the tip if
was a branch). IIUC, this thing never returns a branch name.

If they should be equivalent (and I think the current implementation
says they are), then the answer to "if ... should do the same?"
becomes "no, we should not error out".

Stepping back a bit, the mild suspicion above says

    $ git checkout HEAD^0
    ... do things ...
    $ git checkout -b temp
    ... do more things ...
    $ git checkout -B @{-1}

that creates a new branch whose name is 40-hex of a commit that
happens to be where we started the whole dance *is* a bug.  No sane
user expects that to happen, and the last step "checkout -B @{-1}"
should result in an error instead [*1*].

I was wondering if "git check-ref-format --branch @{-1}", when used
in place of "checkout -B @{-1}" in the above sequence,

I guess you mean '... "git checkout -B $(git check-ref-format --branch
@{-1}", when used in place of "git checkout -B @{-1}" ...' ?

 should or
should not fail.  It really boils down to this question: what @{-1}
expands to and what the user wants to do with it.

In the context of getting a revision (i.e. "rev-parse @{-1}") where
we are asking what the object name is, the value of the detached
HEAD we were on previously is a valid answer we are "expanding @{-1}
to".  If we were on a concrete branch and @{-1} yields a concrete
branch name, then rev-parse would turn that into an object name, and
in the end, in either case, the object name is what we wanted to
get.  So we do not want to error this out.

Right.

But a user of "git check-ref-format --branch" is not asking about
the object name ("git rev-parse" would have been used otherwise).
Which argues for erroring out "check-ref-format --branch @{-1}" if
we were not on a branch in the previous state.

Exactly what I thought.

And that argues for erroring out "check-ref-format --branch @{-1}"
in such a case, i.e. declaring that the first two commands in this
message are not equivalent.

As said before, IIUC, they give equivalent output to stdout only when
the previous thing checked out was not a branch.


-- 
Kaartic