From: Jean-Noël Avila Date: Wed, 27 Oct 2021 13:01:04 GMT Subject: Re: [PATCH] doc: fix grammar rules in commands'syntax Message-ID: <76e58ef5-62c4-ae9e-2557-40345734de25@free.fr> In-Reply-To: On Tue, Oct 26, 2021, Eric Sunshine wrote: > On Tue, Oct 26, 2021 at 11:11 AM Jean-Noël Avila via GitGitGadget > wrote: >> doc: fix grammar rules in commands'syntax > > Missing space. > >> According to the coding guidelines, the placeholders must: >> * be in small letters >> * enclosed in angle brackets >> >> Some other rules are also applied. > > Perhaps just mention them here? > > * use hyphens rather than underscores or spaces > between words There are a lot more places with spaces within placeholders. Will extend. > * indicate repetition with `...` rather than `*` > > were some that I saw while reading. > > Overall, the patch looks good. One or two notes below... Thanks for taking time to review this cumbersome patch. > >> Signed-off-by: Jean-Noël Avila >> --- >> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt >> @@ -11,7 +11,7 @@ SYNOPSIS >> -'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] [] >> +'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] [] > > Nice to see this attention to detail. > >> @@ -43,7 +43,7 @@ You could omit ``, in which case the command degenerates to >> -'git checkout' -b|-B []:: >> +'git checkout' -b|-B []:: > > Likewise. > >> @@ -145,11 +145,11 @@ as `ours` (i.e. "our shared canonical history"), while what you did >> --b :: >> +-b :: >> Create a new branch named `` and start it at >> ``; see linkgit:git-branch[1] for details. > > The names in the description need fixing too: s/_/-/g Ack > >> --B :: >> +-B :: >> Creates the branch `` and start it at ``; >> if it already exists, then reset it to ``. This is >> equivalent to running "git branch" with "-f"; see > > Likewise: s/_/-/g Ack > >> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt >> @@ -9,20 +9,20 @@ git-config - Get and set repository or global options >> -'git config' [] [--type=] [-z|--null] --get-urlmatch name URL >> +'git config' [] [--type=] [-z|--null] --get-urlmatch > > The commit message talks about using lowercase, so perhaps? s/URL/url/ URL is not a word, but an acronym. Lowercasing it looks weird, but that's personal taste. URL appears with a lot different casings across the documents. > >> @@ -102,7 +102,7 @@ OPTIONS >> ---get-urlmatch name URL:: >> +--get-urlmatch :: >> When given a two-part name section.key, the value for >> section..key whose part matches the best to the >> given URL is returned (if no such key exists, the value for > > Ditto. In fact, lowercase is already used in the description, > but not in the item line. > > If wanting to match other documentation files, this would also be > typeset as `` rather than in the description text, but that > change may be well outside the scope of this patch. > >> @@ -145,7 +145,7 @@ See also <>. >> --f config-file:: >> +-f :: >> --file config-file:: > > Need to apply brackets around `config-file` for the `--file` option > too, just as you did for short `-f`. > >> @@ -246,7 +246,7 @@ Valid ``'s include: >> ---get-colorbool name [stdout-is-tty]:: >> +--get-colorbool []:: >> >> Find the color setting for `name` (e.g. `color.diff`) and output >> "true" or "false". `stdout-is-tty` should be either "true" or > > Should you wrap `stdout-is-tty` within angle brackets within the > description too? > The rules for referring to placeholder in text wasn't defined (use angle brackets or not, use monospace or not) . Usage in manpages is diverse. Logically this would be monospace for any token of the synopsis and angle bracket when it's a placeholder. However, this is a larger fixup in manpages, that would require its own patch. >> @@ -257,7 +257,7 @@ Valid ``'s include: >> ---get-color name [default]:: >> +--get-color []:: >> >> Find the color configured for `name` (e.g. `color.diff.new`) and >> output it as the ANSI color escape sequence to the standard > > And here? rather than `name`? > >> diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt >> @@ -8,7 +8,7 @@ git-credential - Retrieve and store user credentials >> -git credential >> +git credential [fill|approve|reject] > > The original was indeed wrong but the revised text is also slightly > misleading. The square brackets suggest that the "action" is optional, > but in fact it's not, so this should be using parentheses: > > git credential (fill|approve|reject) > > Also, the usage text in builtin/credential.c is wrong: > > % git credential > usage: git credential [fill|approve|reject] > > It should be using parentheses, as well, but fixing that may be > outside the scope of this patch (and can be done later). > >> diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt >> @@ -9,11 +9,11 @@ git-cvsimport - Salvage your data out of another SCM people love to hate >> -'git cvsimport' [-o ] [-h] [-v] [-d ] >> +'git cvsimport' [-o ] [-h] [-v] [-d ] >> [-A ] [-p ] [-P ] >> - [-C ] [-z ] [-i] [-k] [-u] [-s ] >> + [-C ] [-z ] [-i] [-k] [-u] [-s ] >> [-a] [-m] [-M ] [-S ] [-L ] >> - [-r ] [-R] [] >> + [-r ] [-R] [] > > I wonder if should be changed to ? > >> diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt >> @@ -12,7 +12,7 @@ SYNOPSIS >> 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs] >> [--[no-]full] [--strict] [--verbose] [--lost-found] >> [--[no-]dangling] [--[no-]progress] [--connectivity-only] >> - [--[no-]name-objects] [