Re: [PATCH v2 4/9] doc: use only hyphens as word separators in placeholders

[Eli Schwartz <eschwartz@archlinux.org>]

On 10/28/21 12:21 PM, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> 
> According to CodingGuidelines, spaces and underscores are not
> allowed in placeholders.


I have a patch under review that touches the same files you are
modifying here. As I've been pointed to these changes, I'd like to make
a quick observation.


> @@ -101,9 +101,9 @@ commits are displayed, but not the way the diff is shown e.g. with
>  `git log --raw`. To get full object names in a raw diff format,
>  use `--no-abbrev`.
>  
> -* 'format:<string>'
> +* 'format:<format-string>'
>  +
> -The 'format:<string>' format allows you to specify which information
> +The 'format:<format-string>' format allows you to specify which information
>  you want to show. It works a little bit like printf format,
>  with the notable exception that you get a newline with '%n'
>  instead of '\n'.
> @@ -273,12 +273,12 @@ endif::git-rev-list[]
>  			  If any option is provided multiple times the
>  			  last occurrence wins.
>  +
> -The boolean options accept an optional value `[=<BOOL>]`. The values
> +The boolean options accept an optional value `[=<value>]`. The values


Here you change "BOOL" to "value", below you change it to "bool-value".


>  `true`, `false`, `on`, `off` etc. are all accepted. See the "boolean"
>  sub-section in "EXAMPLES" in linkgit:git-config[1]. If a boolean
>  option is given with no value, it's enabled.
>  +
> -** 'key=<K>': only show trailers with specified key. Matching is done
> +** 'key=<key>': only show trailers with specified <key>. Matching is done
>     case-insensitively and trailing colon is optional. If option is
>     given multiple times trailer lines matching any of the keys are
>     shown. This option automatically enables the `only` option so that
> @@ -286,9 +286,9 @@ option is given with no value, it's enabled.
>     desired it can be disabled with `only=false`.  E.g.,
>     `%(trailers:key=Reviewed-by)` shows trailer lines with key
>     `Reviewed-by`.
> -** 'only[=<BOOL>]': select whether non-trailer lines from the trailer
> +** 'only[=<bool-value>]': select whether non-trailer lines from the trailer
>     block should be included.
> -** 'separator=<SEP>': specify a separator inserted between trailer
> +** 'separator=<sep>': specify a separator inserted between trailer
>     lines. When this option is not given each trailer line is
>     terminated with a line feed character. The string SEP may contain
>     the literal formatting codes described above. To use comma as
> @@ -296,15 +296,15 @@ option is given with no value, it's enabled.
>     next option. E.g., `%(trailers:key=Ticket,separator=%x2C )`
>     shows all trailer lines whose key is "Ticket" separated by a comma
>     and a space.
> -** 'unfold[=<BOOL>]': make it behave as if interpret-trailer's `--unfold`
> +** 'unfold[=<bool-value>]': make it behave as if interpret-trailer's `--unfold`
>     option was given. E.g.,
>     `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
> -** 'keyonly[=<BOOL>]': only show the key part of the trailer.
> -** 'valueonly[=<BOOL>]': only show the value part of the trailer.
> -** 'key_value_separator=<SEP>': specify a separator inserted between
> +** 'keyonly[=<bool-value>]': only show the key part of the trailer.
> +** 'valueonly[=<bool-value>]': only show the value part of the trailer.
> +** 'key_value_separator=<sep>': specify a separator inserted between
>     trailer lines. When this option is not given each trailer key-value
>     pair is separated by ": ". Otherwise it shares the same semantics
> -   as 'separator=<SEP>' above.
> +   as 'separator=<sep>' above.
>  
>  NOTE: Some placeholders may depend on other options given to the
>  revision traversal engine. For example, the `%g*` reflog options will


These changes over here are contrary to the statement in the commit
message. In addition to switching to hyphens, they:

- switch casing (okay, makes sense, you point this out in the cover
  letter but maybe it's worth mentioning it in the commit message too?
  idk)

- change the terms used -- and this I don't understand. I'm not really
  bothered by switching <n> to <number> or <k> to <key>, as these
  changes seem reasonable (though again, they are not mentioned in the
  commit message). However, "bool-value" seems odd. Why that and not
  "number-value"? IMHO the "value" is redundant here, let it be "bool"
  and "number".

  Similarly "the 'format:<format-string>' format" feels highly
  redundant, I expect the reader knows that <string> contains a format
  inside it as it's mentioned immediately before *and* after.

-- 
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User