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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sep 1, 2017, 23:19 UTC
Message-ID: <20170901231933.GC143138@aiede.mtv.corp.google.com>
In-Reply-To: <20170901144931.26114-1-me@ikke.info>

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.

Show 16 quoted lines

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