[PATCH] doc/for-each-ref: explicitly specify option names

[PATCH] doc/for-each-ref: explicitly specify option names

[Kevin Daudt]

For count, sort and format, only the argument names were listed under
OPTIONS, not the option names.

Add the option names to make it clear the options exist

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 Documentation/git-for-each-ref.txt | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index bb370c9c7..0c2032855 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -25,19 +25,25 @@ host language allowing their direct evaluation in that language.
 
 OPTIONS
 -------
-<count>::
+<pattern>...::
+	If one or more patterns are given, only refs are shown that
+	match against at least one pattern, either using fnmatch(3) or
+	literally, in the latter case matching completely or from the
+	beginning up to a slash.
+
+--count <count>::
 	By default the command shows all refs that match
 	`<pattern>`.  This option makes it stop after showing
 	that many refs.
 
-<key>::
+--sort <key>::
 	A field name to sort on.  Prefix `-` to sort in
 	descending order of the value.  When unspecified,
 	`refname` is used.  You may use the --sort=<key> option
 	multiple times, in which case the last key becomes the primary
 	key.
 
-<format>::
+--format <format>::
 	A string that interpolates `%(fieldname)` from a ref being shown
 	and the object it points at.  If `fieldname`
 	is prefixed with an asterisk (`*`) and the ref points
@@ -50,12 +56,6 @@ OPTIONS
 	`xx`; for example `%00` interpolates to `\0` (NUL),
 	`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
 
-<pattern>...::
-	If one or more patterns are given, only refs are shown that
-	match against at least one pattern, either using fnmatch(3) or
-	literally, in the latter case matching completely or from the
-	beginning up to a slash.
-
 --shell::
 --perl::
 --python::
-- 
2.14.1.459.g238e487ea9

Re: [PATCH] doc/for-each-ref: explicitly specify option names

[Jeff King]

On Fri, Sep 01, 2017 at 04:49:31PM +0200, Kevin Daudt wrote:

> For count, sort and format, only the argument names were listed under
> OPTIONS, not the option names.
> 
> Add the option names to make it clear the options exist

Yeah, this looks much better. I could see having a general "<format>"
section if it was used as the argument to multiple options, but that's
not what's going on here. And even if it were, the right thing is to
have individual "--foo=<format>" and "--bar=<format>" items in the list,
and let them refer to a more general section on formats.

-Peff

Re: [PATCH] doc/for-each-ref: explicitly specify option names

[Johannes Schindelin]

Hi Kevin,

On Fri, 1 Sep 2017, Kevin Daudt wrote:

> For count, sort and format, only the argument names were listed under
> OPTIONS, not the option names.
> 
> Add the option names to make it clear the options exist
> 
> Signed-off-by: Kevin Daudt <me@ikke.info>
> ---

This patch makes sense to me.

Thanks,
Johannes

Re: [PATCH] doc/for-each-ref: explicitly specify option names

[Jonathan Nieder]

Kevin Daudt wrote:

> For count, sort and format, only the argument names were listed under
> OPTIONS, not the option names.
>
> Add the option names to make it clear the options exist

nit: missing full-stop (.) at end of sentence.

> Signed-off-by: Kevin Daudt <me@ikke.info>
> ---
>  Documentation/git-for-each-ref.txt | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Makes sense.

> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index bb370c9c7..0c2032855 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -25,19 +25,25 @@ host language allowing their direct evaluation in that language.
>  
>  OPTIONS
>  -------
> +<pattern>...::
> +	If one or more patterns are given, only refs are shown that
> +	match against at least one pattern, either using fnmatch(3) or
> +	literally, in the latter case matching completely or from the
> +	beginning up to a slash.
> +
> -<count>::
> +--count <count>::

nit: the usage string (and "git help cli") recommends --count=<count>
with equal-sign, so it probably makes sense to match that (and likewise
for the other options).

Looking closer reveals more problems with the manpage:

* the synopsis mixes the style using = and the style using space,
  without a clear reason for doing so

* the synopsis implies that I can run
  "git for-each-ref --merged --contains HEAD".  But that produces

	fatal: malformed object name --contains

  An exact grammar would be harder to read than what is here, but
  perhaps it's worth a word or two on that subject in the description
  section.

  The description of --contains in git-branch.txt has the same problem.
  It's tempting to treat the <commit> argument to --contains as
  non-optional in the synopsis, since it's always harmless for a user
  to specify it.  The OPTIONS section can still explain what happens
  when <commit> isn't specified.

* by the way, the synopsis and options sections use <object> where
  they mean <commit>.

How about something like this patch, for squashing in?  It focuses on
just the ordering and option name issues described in the commit
message --- it doesn't make any of the more aggressive changes
described above.

Thanks,
Jonathan

diff --git i/Documentation/git-for-each-ref.txt w/Documentation/git-for-each-ref.txt
index 0c20328555..66b4e0a405 100644
--- i/Documentation/git-for-each-ref.txt
+++ w/Documentation/git-for-each-ref.txt
@@ -10,8 +10,9 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
-		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
-		   [--contains [<object>]] [--no-contains [<object>]]
+		   [--points-at=<object>]
+		   (--merged[=<object>] | --no-merged[=<object>])
+		   [--contains[=<object>]] [--no-contains[=<object>]]
 
 DESCRIPTION
 -----------
@@ -31,19 +32,19 @@ OPTIONS
 	literally, in the latter case matching completely or from the
 	beginning up to a slash.
 
---count <count>::
+--count=<count>::
 	By default the command shows all refs that match
 	`<pattern>`.  This option makes it stop after showing
 	that many refs.
 
---sort <key>::
+--sort=<key>::
 	A field name to sort on.  Prefix `-` to sort in
 	descending order of the value.  When unspecified,
 	`refname` is used.  You may use the --sort=<key> option
 	multiple times, in which case the last key becomes the primary
 	key.
 
---format <format>::
+--format=<format>::
 	A string that interpolates `%(fieldname)` from a ref being shown
 	and the object it points at.  If `fieldname`
 	is prefixed with an asterisk (`*`) and the ref points
@@ -65,24 +66,24 @@ OPTIONS
 	the specified host language.  This is meant to produce
 	a scriptlet that can directly be `eval`ed.
 
---points-at <object>::
+--points-at=<object>::
 	Only list refs which points at the given object.
 
---merged [<object>]::
+--merged[=<object>]::
 	Only list refs whose tips are reachable from the
 	specified commit (HEAD if not specified),
 	incompatible with `--no-merged`.
 
---no-merged [<object>]::
+--no-merged[=<object>]::
 	Only list refs whose tips are not reachable from the
 	specified commit (HEAD if not specified),
 	incompatible with `--merged`.
 
---contains [<object>]::
+--contains[=<object>]::
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
---no-contains [<object>]::
+--no-contains[=<object>]::
 	Only list refs which don't contain the specified commit (HEAD
 	if not specified).
 

Re: [PATCH] doc/for-each-ref: explicitly specify option names

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> For count, sort and format, only the argument names were listed under
> OPTIONS, not the option names.
>
> Add the option names to make it clear the options exist
>
> Signed-off-by: Kevin Daudt <me@ikke.info>
> ---
>  Documentation/git-for-each-ref.txt | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Sounds sensible.  First I thought this was done deliberately because
these placeholder can apply to more than one option and we wanted to
explain each thing only once (e.g. "<object>" appears in 5 places,
and having to repeat something like "you can spell <object> by the
unique prefix of the object name, or the name of a ref that points
at it, or ..." in each option would be awkward), but that is not the
case here.

While we are at it, I just noticed that the SYNOPSIS section makes
it look as if <pattern>... must come before points-at, merged, and
their friends, but I do not think that should be the case.  I also
notice that unlike --sort/--format/... the last four/five options
are spelled with "--option <value>" syntax; we should consistently
use "--option=<value>" instead there.

But these two are separate issues that can be fixed in a patch
separate from this one I am responding to.

p.s.  I am still mostly offline and won't be doing any reviews or
queuing that requires me to look at anything beyond the patch
context.  Please do not get disappointed if you do not see your
patch in my tree until later next week.

[PATCH v2 1/2] doc/for-each-ref: consistently use '=' to between argument names and values

[Kevin Daudt]

The synopsis and description inconsistently add a '=' between the
argument name and it's value. Make this consistent.

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 Documentation/git-for-each-ref.txt | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index bb370c9c7..1015c88f6 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -10,8 +10,9 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
-		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
-		   [--contains [<object>]] [--no-contains [<object>]]
+		   [--points-at=<object>]
+		   (--merged[=<object>] | --no-merged[=<object>])
+		   [--contains[=<object>]] [--no-contains[=<object>]]
 
 DESCRIPTION
 -----------
@@ -65,24 +66,24 @@ OPTIONS
 	the specified host language.  This is meant to produce
 	a scriptlet that can directly be `eval`ed.
 
---points-at <object>::
+--points-at=<object>::
 	Only list refs which points at the given object.
 
---merged [<object>]::
+--merged[=<object>]::
 	Only list refs whose tips are reachable from the
 	specified commit (HEAD if not specified),
 	incompatible with `--no-merged`.
 
---no-merged [<object>]::
+--no-merged[=<object>]::
 	Only list refs whose tips are not reachable from the
 	specified commit (HEAD if not specified),
 	incompatible with `--merged`.
 
---contains [<object>]::
+--contains[=<object>]::
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
---no-contains [<object>]::
+--no-contains[=<object>]::
 	Only list refs which don't contain the specified commit (HEAD
 	if not specified).
 
-- 
2.14.1.459.g238e487ea9

[PATCH v2 2/2] doc/for-each-ref: explicitly specify option names

[Kevin Daudt]

For count, sort and format, only the argument names were listed under
OPTIONS, not the option names.

Add the option names to make it clear the options exist

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 Documentation/git-for-each-ref.txt | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 1015c88f6..66b4e0a40 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -26,19 +26,25 @@ host language allowing their direct evaluation in that language.
 
 OPTIONS
 -------
-<count>::
+<pattern>...::
+	If one or more patterns are given, only refs are shown that
+	match against at least one pattern, either using fnmatch(3) or
+	literally, in the latter case matching completely or from the
+	beginning up to a slash.
+
+--count=<count>::
 	By default the command shows all refs that match
 	`<pattern>`.  This option makes it stop after showing
 	that many refs.
 
-<key>::
+--sort=<key>::
 	A field name to sort on.  Prefix `-` to sort in
 	descending order of the value.  When unspecified,
 	`refname` is used.  You may use the --sort=<key> option
 	multiple times, in which case the last key becomes the primary
 	key.
 
-<format>::
+--format=<format>::
 	A string that interpolates `%(fieldname)` from a ref being shown
 	and the object it points at.  If `fieldname`
 	is prefixed with an asterisk (`*`) and the ref points
@@ -51,12 +57,6 @@ OPTIONS
 	`xx`; for example `%00` interpolates to `\0` (NUL),
 	`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
 
-<pattern>...::
-	If one or more patterns are given, only refs are shown that
-	match against at least one pattern, either using fnmatch(3) or
-	literally, in the latter case matching completely or from the
-	beginning up to a slash.
-
 --shell::
 --perl::
 --python::
-- 
2.14.1.459.g238e487ea9

Re: [PATCH v2 1/2] doc/for-each-ref: consistently use '=' to between argument names and values

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> The synopsis and description inconsistently add a '=' between the
> argument name and it's value. Make this consistent.
>
> Signed-off-by: Kevin Daudt <me@ikke.info>
> ---
>  Documentation/git-for-each-ref.txt | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

Good idea, and I think it is in line with an earlier suggestion by
Jonathan (cc'ed).

Thanks.

> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index bb370c9c7..1015c88f6 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -10,8 +10,9 @@ SYNOPSIS
>  [verse]
>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>  		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
> -		   [--points-at <object>] [(--merged | --no-merged) [<object>]]
> -		   [--contains [<object>]] [--no-contains [<object>]]
> +		   [--points-at=<object>]
> +		   (--merged[=<object>] | --no-merged[=<object>])
> +		   [--contains[=<object>]] [--no-contains[=<object>]]
>  
>  DESCRIPTION
>  -----------
> @@ -65,24 +66,24 @@ OPTIONS
>  	the specified host language.  This is meant to produce
>  	a scriptlet that can directly be `eval`ed.
>  
> ---points-at <object>::
> +--points-at=<object>::
>  	Only list refs which points at the given object.
>  
> ---merged [<object>]::
> +--merged[=<object>]::
>  	Only list refs whose tips are reachable from the
>  	specified commit (HEAD if not specified),
>  	incompatible with `--no-merged`.
>  
> ---no-merged [<object>]::
> +--no-merged[=<object>]::
>  	Only list refs whose tips are not reachable from the
>  	specified commit (HEAD if not specified),
>  	incompatible with `--merged`.
>  
> ---contains [<object>]::
> +--contains[=<object>]::
>  	Only list refs which contain the specified commit (HEAD if not
>  	specified).
>  
> ---no-contains [<object>]::
> +--no-contains[=<object>]::
>  	Only list refs which don't contain the specified commit (HEAD
>  	if not specified).

Re: [PATCH v2 1/2] doc/for-each-ref: consistently use '=' to between argument names and values

[Kevin Daudt]

On Tue, Sep 12, 2017 at 11:28:00AM +0900, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
> 
> > The synopsis and description inconsistently add a '=' between the
> > argument name and it's value. Make this consistent.
> >
> > Signed-off-by: Kevin Daudt <me@ikke.info>
> > ---
> >  Documentation/git-for-each-ref.txt | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> Good idea, and I think it is in line with an earlier suggestion by
> Jonathan (cc'ed).
> 
> Thanks.
> 

Yeah, this is his diff applied. Forgot to CC him.