Thread (35 messages) 35 messages, 6 authors, 2021-11-07

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

flat view

From: Eli Schwartz <hidden>
Date: 2021-10-31 18:59:18

On 10/28/21 12:21 PM, Jean-Noël Avila via GitGitGadget wrote:
From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <redacted>

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.

quoted hunk ↗ jump to hunk
@@ -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".

quoted hunk ↗ jump to hunk
 `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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help