diff mbox series

[2/5] pretty format %(trailers): avoid needless repetition

Message ID 20201205013918.18981-3-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Ævar Arnfjörð Bjarmason Dec. 5, 2020, 1:39 a.m. UTC
Change the documentation for the various %(trailers) options so it
isn't repeating part of the documentation for "only" about how boolean
values are handled. Instead let's split the description of that into
general documentation at the top.

It then suffices to refer to it by listing the options as
"opt[=bool]". I'm also changing it to "[=bool]" from "[=val]". It took
me a couple of readings to realize that while to realize that these
options were referring back to the "only" option's treatment of
boolean values. Let's try to make this more explicit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/pretty-formats.txt | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Comments

Christian Couder Dec. 5, 2020, 5:43 a.m. UTC | #1
On Sat, Dec 5, 2020 at 2:39 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
> Change the documentation for the various %(trailers) options so it
> isn't repeating part of the documentation for "only" about how boolean
> values are handled. Instead let's split the description of that into
> general documentation at the top.

Great!

> It then suffices to refer to it by listing the options as
> "opt[=bool]". I'm also changing it to "[=bool]" from "[=val]".

Nice! I wonder if "[=<bool>]" or "[=<BOOL>]" might be even better as
we use "=<K>" for key and "=<SEP>" for separator.

> It took
> me a couple of readings to realize that while to realize that these
> options were referring back to the "only" option's treatment of
> boolean values. Let's try to make this more explicit.

Yeah, it's definitely an improvement.

> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -252,7 +252,14 @@ endif::git-rev-list[]
>                           interpreted by
>                           linkgit:git-interpret-trailers[1]. The
>                           `trailers` string may be followed by a colon
> -                         and zero or more comma-separated options:
> +                         and zero or more comma-separated options.
> ++
> +The boolean options accept an optional value. The values `true`,

Maybe: s/an optional value./an optional value `[=<BOOL>]`./

> +`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.

s/value it's enabled/value, it's enabled/

> +If any option is provided multiple times the last occurance wins.

It might be better to have this sentence before the above paragraph
about boolean options, as it's more general.

Thanks,
Christian.
diff mbox series

Patch

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 54f793d424f..8e066594624 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -252,7 +252,14 @@  endif::git-rev-list[]
 			  interpreted by
 			  linkgit:git-interpret-trailers[1]. The
 			  `trailers` string may be followed by a colon
-			  and zero or more comma-separated options:
+			  and zero or more comma-separated options.
++
+The boolean options accept an optional value. The values `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. If any option is provided
+multiple times the last occurance wins.
++
 ** 'key=<K>': 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
@@ -264,27 +271,21 @@  endif::git-rev-list[]
    desired it can be disabled with `only=false`.  E.g.,
    `%(trailers:key=Reviewed-by)` shows trailer lines with key
    `Reviewed-by`.
-** 'only[=val]': select whether non-trailer lines from the trailer
-   block should be included. The `only` keyword may optionally be
-   followed by an equal sign and one of `true`, `on`, `yes` to omit or
-   `false`, `off`, `no` to show the non-trailer lines. If option is
-   given without value it is enabled. If given multiple times the last
-   value is used.
+** 'only[=bool]': select whether non-trailer lines from the trailer
+   block should be included.
 ** '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
    separator one must use `%x2C` as it would otherwise be parsed as
-   next option. If separator option is given multiple times only the
-   last one is used. E.g., `%(trailers:key=Ticket,separator=%x2C )`
+   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[=val]': make it behave as if interpret-trailer's `--unfold`
-   option was given. In same way as to for `only` it can be followed
-   by an equal sign and explicit value. E.g.,
+** 'unfold[=bool]': make it behave as if interpret-trailer's `--unfold`
+   option was given. E.g.,
    `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
-** 'valueonly[=val]': skip over the key part of the trailer line and only
-   show the value part. Also this optionally allows explicit value.
+** 'valueonly[=bool]': skip over the key part of the trailer line and only
+   show the value part.
 
 NOTE: Some placeholders may depend on other options given to the
 revision traversal engine. For example, the `%g*` reflog options will