mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <>
To: Kaartic Sivaraam <>
Cc: Git mailing list <>
Subject: Re: [PATCH v2 2/2] Doc/check-ref-format: clarify information about @{-N} syntax
Date: Sat, 02 Dec 2017 17:52:02 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Kaartic Sivaraam's message of "Tue, 28 Nov 2017 20:13:57 +0530")

Kaartic Sivaraam <> writes:

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

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

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

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.

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.


*1* "It should instead detach HEAD at that commit if @{-1} refers to
    a detached HEAD state" is not a good suggestion (we wouldn't
    have "-B" if a mere branch switching is desired).

  reply	other threads:[~2017-12-03  1:52 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-19 17:54 [PATCH] docs: checking out using @{-N} can lead to detached state Kaartic Sivaraam
2017-11-20  2:09 ` Junio C Hamano
2017-11-20 15:18   ` Kaartic Sivaraam
2017-11-27 17:28   ` [PATCH v2 1/2] Doc/checkout: " Kaartic Sivaraam
2017-11-27 17:28     ` [PATCH v2 2/2] Doc/check-ref-format: clarify information about @{-N} syntax Kaartic Sivaraam
2017-11-28  2:40       ` Junio C Hamano
2017-11-28 14:43         ` Kaartic Sivaraam
2017-12-03  1:52           ` Junio C Hamano [this message]
2017-12-04 17:25             ` Kaartic Sivaraam
2017-12-04 18:44               ` Junio C Hamano
2017-12-05  5:20                 ` Kaartic Sivaraam
2017-11-28 16:34         ` [PATCH v3 " Kaartic Sivaraam
2017-12-03  2:08           ` Junio C Hamano
2017-12-06 17:58             ` Kaartic Sivaraam
2017-12-14 12:30             ` [PATCH v4 " Kaartic Sivaraam
2017-12-14 18:02               ` Junio C Hamano
2017-12-16  5:38                 ` Kaartic Sivaraam
2017-12-16  8:13                 ` [PATCH v5 0/1] clarify about @{-N} syntax in check-ref-format documentation Kaartic Sivaraam
2017-12-16  8:13                   ` [PATCH v5 1/1] Doc/check-ref-format: clarify information about @{-N} syntax Kaartic Sivaraam
2017-11-28  2:33     ` [PATCH v2 1/2] Doc/checkout: checking out using @{-N} can lead to detached state Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).