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

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Dec 5, 2017, 05:20 UTC
Message-ID: <55d5598e-7c4e-c4ea-53a4-7fece0ea4d38@gmail.com>
In-Reply-To: <xmqq8teimiz3.fsf@gitster.mtv.corp.google.com>

On Tuesday 05 December 2017 12:14 AM, Junio C Hamano wrote:

Show 29 quoted lines

> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
>>> 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}" ...' ?
> 
> No you guessed wrong.  I was (and am) wondering if the last step in
> the following sequence should fail.
>>>      $ git checkout HEAD^0
>>>      ... do things ...
>>>      $ git checkout -b temp
>>>      ... do more things ...
>>>      $ git check-ref-format --branch @{-1}
>

Ok. Now I get what you say.

> And I am leaning towards saying that it is a bug that it does not
> fail; @{-1} is a detached HEAD and not a concrete branch name in
> this case,

It seems your thought is similar to the following thought that I expressed in [1],

-- 8< --

Show 7 quoted lines

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

-- >8 --

> so "check-ref-format --branch" should at least notice
> and say that it is a request that may lead to a nonsense next step
> (which is to create a branch with that 40-hex name).
>

Makes sense, this should at least be noted in the Documentation. Is that what you had in mind too or do you expect 'check-ref-format' to do something else too?

[1]: https://public-inbox.org/git/1511880237.10193.5.camel@gmail.com/

--- Kaartic