All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Cc: Git mailing list <git@vger.kernel.org>
Subject: Re: [PATCH v4 2/2] Doc/check-ref-format: clarify information about @{-N} syntax
Date: Thu, 14 Dec 2017 10:02:01 -0800	[thread overview]
Message-ID: <xmqqa7ylur1i.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171214123027.9105-1-kaartic.sivaraam@gmail.com> (Kaartic Sivaraam's message of "Thu, 14 Dec 2017 18:00:27 +0530")

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

>  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
> -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.
> +it can be used as a valid branch name e.g. when creating a new branch
> +(but be cautious when using the previous checkout syntax; it may refer
> +to a detached HEAD state). The rule `git check-ref-format --branch
> +$name` implements 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 checkout syntax''
> +`@{-n}`.  For example, `@{-1}` is a way to refer the last thing that
> +was checked out using "git checkout" operation. 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. As an
> +exception note that, the ``previous checkout operation'' might result
> +in a commit object name when the N-th last thing checked out was not
> +a branch.

Looks alright.  

It was made unnecessarily harder to review because it was marked as
2/2, even though this no longer applies on top of the copy of 1/2
that was merged some time ago.  I needed to find that it was rebased
on top of 'master'; it wouldn't have been necessary if this was sent
as a single patch (with comment saying that this used to be 2/2
whose first one already graduated to 'master' under the three-dash
line).

Also re-wrapping the lines only to squeeze in "but be cautious..."
and replace s/branch/checkout/ in a few places did not help to make
it easy to spot what's changed.

This part looked a bit strange.

> +it can be used as a valid branch name e.g. when creating a new branch
> +(but be cautious when using the previous checkout syntax; it may refer
> +to a detached HEAD state). The rule `git check-ref-format --branch

I think "e.g. when creating a new branch" is a parenthetical remark,
so it should be inside parenthesis.  As the last three lines in the
new text (quoted above) already warns that it may not be a branch name,
I am not sure if the "but be cautious" adds much value, though.

  reply	other threads:[~2017-12-14 18:02 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
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 [this message]
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=xmqqa7ylur1i.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kaartic.sivaraam@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.