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

[Kaartic Sivaraam]

On Thursday 14 December 2017 11:32 PM, Junio C Hamano wrote:

Kaartic Sivaraam  writes:

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.


Sorry about that but I don't remember doing anything that made it not to 
apply on top of 1/2. (I just amended my changes to my topic branch. It 
can be found at [1])




I needed to find that it was rebased
on top of 'master';


I don't remember doing any rebase on top of 'master'. My topic was (and 
still is) based on the 'master' when it was pointing at 89ea799ff (Sync 
with maint, 2017-11-15). Anyways, it's my mistake as I didn't specify 
the branch on which I based this. Sorry about that.





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.



I expected this would happen but I thought the line shouldn't grew too 
much so that they have to re-wrapped. Seems it would have been better if 
I did the re-wrapping as a follow-up commit (didn't strike me then).




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.


It was. I brought them out to introduce the parenthetical warning. I'll 
send a v5 by putting the remark back inside parantheses and bringing the 
warning out. If it's not ok, let me know. I'll also try to do the 
re-wrapping as a separate cleanup patch.




 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.



That warning was for the impatient readers, who might want to find quick 
answers as to why they saw an odd behaviour (check-ref-froamt --branch 
not failing for a commit object name) (or) those who would want to use 
'check-ref-format --branch' but do not find time to read the whole 
paragraph.

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

[Junio C Hamano]

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

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

[Kaartic Sivaraam]

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 a commit object name if the previous check
out was a detached HEAD state; or a branch name, otherwise. 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.

Signed-off-by: Kaartic Sivaraam 
---

Changes in v4:

- updated the commit message
- made changes suggested by Junio

 Documentation/git-check-ref-format.txt | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-check-ref-format.txt 
b/Documentation/git-check-ref-format.txt
index cf0a0b7df..8172a6b9a 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -78,17 +78,21 @@ 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
-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.
 
 OPTIONS
 ---
@@ -116,7 +120,7 @@ OPTIONS
 EXAMPLES
 
 
-* Print the name of the previous branch:
+* Print the name of the previous thing checked out:
 +
 
 $ git check-ref-format --branch @{-1}
-- 
2.15.0.531.g2ccb3012c