Re: [PATCH] doc: fix grammar rules in commands'syntax

On Tue, Oct 26, 2021, Eric Sunshine wrote:

On Tue, Oct 26, 2021 at 11:11 AM Jean-Noël Avila via GitGitGadget
[off-list ref] wrote:

quoted

doc: fix grammar rules in commands'syntax

Missing space.

quoted

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.

quoted

Signed-off-by: Jean-Noël Avila <redacted>
---

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] <new_branch>] [<start_point>]
+'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new-branch>] [<start-point>]

Nice to see this attention to detail.

quoted

@@ -43,7 +43,7 @@ You could omit `<branch>`, in which case the command degenerates to
-'git checkout' -b|-B <new_branch> [<start point>]::
+'git checkout' -b|-B <new-branch> [<start-point>]::

Likewise.

quoted

@@ -145,11 +145,11 @@ as `ours` (i.e. "our shared canonical history"), while what you did
--b <new_branch>::
+-b <new-branch>::
        Create a new branch named `<new_branch>` and start it at
        `<start_point>`; see linkgit:git-branch[1] for details.

The names in the description need fixing too: s/_/-/g

Ack

quoted

--B <new_branch>::
+-B <new-branch>::
        Creates the branch `<new_branch>` and start it at `<start_point>`;
        if it already exists, then reset it to `<start_point>`. This is
        equivalent to running "git branch" with "-f"; see

Likewise: s/_/-/g

Ack

quoted

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' [<file-option>] [--type=<type>] [-z|--null] --get-urlmatch name URL
+'git config' [<file-option>] [--type=<type>] [-z|--null] --get-urlmatch <name> <URL>

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.

quoted

@@ -102,7 +102,7 @@ OPTIONS
---get-urlmatch name URL::
+--get-urlmatch <name> <URL>::
        When given a two-part name section.key, the value for
        section.<url>.key whose <url> part matches the best to the
        given URL is returned (if no such key exists, the value for

Ditto. In fact, lowercase <url> 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 `<url>` rather than <url> in the description text, but that
change may be well outside the scope of this patch.

quoted

@@ -145,7 +145,7 @@ See also <<FILES>>.
--f config-file::
+-f <config-file>::
 --file config-file::

Need to apply brackets around `config-file` for the `--file` option
too, just as you did for short `-f`.

quoted

@@ -246,7 +246,7 @@ Valid `<type>`'s include:
---get-colorbool name [stdout-is-tty]::
+--get-colorbool <name> [<stdout-is-tty>]::

        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.

quoted

@@ -257,7 +257,7 @@ Valid `<type>`'s include:
---get-color name [default]::
+--get-color <name> [<default>]::

        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? <name> rather than `name`?

quoted

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 <fill|approve|reject>
+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).

quoted

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 <branch-for-HEAD>] [-h] [-v] [-d <CVSROOT>]
+'git cvsimport' [-o <branch-for-HEAD>] [-h] [-v] [-d <cvsroot>]
              [-A <author-conv-file>] [-p <options-for-cvsps>] [-P <file>]
-             [-C <git_repository>] [-z <fuzz>] [-i] [-k] [-u] [-s <subst>]
+             [-C <git-repository>] [-z <fuzz>] [-i] [-k] [-u] [-s <subst>]
              [-a] [-m] [-M <regex>] [-S <regex>] [-L <commitlimit>]
-             [-r <remote>] [-R] [<CVS_module>]
+             [-r <remote>] [-R] [<CVS-module>]

I wonder if <commitlimit> should be changed to <commit-limit>?

quoted

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] [<object>*]
+        [--[no-]name-objects] [<object>...]

Okay.

quoted

diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt

@@ -79,7 +79,7 @@ repository.  If not specified, fall back to the default name (currently
---shared[=(false|true|umask|group|all|world|everybody|0xxx)]::
+--shared[=(false|true|umask|group|all|world|everybody|0<octal>)]::

This feels slightly unusual; I'd have expected just plain `<octal>`
without the leading `0`, and...

quoted

@@ -110,13 +110,14 @@ the repository permissions.
-'0xxx'::
+'0<octal>'::

.. this would also say just `<octal>`, and...

quoted

-'0xxx' is an octal number and each file will have mode '0xxx'. '0xxx' will
-override users' umask(2) value (and not only loosen permissions as 'group' and
-'all' does). '0640' will create a repository which is group-readable, but not
-group-writable or accessible to others. '0660' will create a repo that is
-readable and writable to the current user and group, but inaccessible to others.
+'0<octal>' is an octal number and each file will have mode
+'0<octal>'. '0<octal>' will override users' umask(2) value (and not
+only loosen permissions as 'group' and 'all' does). '0640' will create
+a repository which is group-readable, but not group-writable or
+accessible to others. '0660' will create a repo that is readable and
+writable to the current user and group, but inaccessible to others.

... this would then go on to explain that `<octal>` "... is an octal
number starting with literal `0`...".

But it's subjective and others might feel differently.

quoted

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt

@@ -81,7 +81,7 @@ produced by `--stat`, etc.
-<revision range>::
+<revision-range>::
        Show only commits in the specified revision range.  When no
        <revision range> is specified, it defaults to `HEAD` (i.e. the
        whole history leading to the current commit).  `origin..HEAD`

Also need to fix the description: s/<revision range>/<revision-range>/

quoted

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt

@@ -10,8 +10,8 @@ SYNOPSIS
 'git ls-files' [-z] [-t] [-v] [-f]
-               (--[cached|deleted|others|ignored|stage|unmerged|killed|modified])*
-               (-[c|d|o|i|s|u|k|m])*
+               [--(cached|deleted|others|ignored|stage|unmerged|killed|modified)...]
+               [-(c|d|o|i|s|u|k|m)...]

I wonder if we could make it easier on users if written like this:

    [--cached|--deleted|--others|--blah|--blah]...
    [-c|-d|-o|-i|-s|-u|-k|-m]...

But that's subjective.

It would better match synopsis of other commands with the following
grammar which expresses alternative options as explained in the
remainder of the manpage:

[-c|--cached] [-d|--deleted] [-m|--modified] and so on.

quoted

diff --git a/Documentation/git-pack-redundant.txt b/Documentation/git-pack-redundant.txt

@@ -9,7 +9,7 @@ git-pack-redundant - Find redundant pack files
-'git pack-redundant' [ --verbose ] [ --alt-odb ] < --all | .pack filename ... >
+'git pack-redundant' [ --verbose ] [ --alt-odb ] ( --all | <.pack-filename>... )

I'd probably drop the leading dot in <.pack-filename>. It shouldn't be
difficult for a reader to figure out that these are the files with
`.pack` extension, and if they do need help understanding that, then
probably better to explain in prose that <pack-filename> is a pack
file with `.pack` extension.

quoted

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt

@@ -89,7 +89,7 @@ counts both authors and co-authors.
-<revision range>::
+<revision-range>::
        Show only commits in the specified revision range.  When no
        <revision range> is specified, it defaults to `HEAD` (i.e. the
        whole history leading to the current commit).  `origin..HEAD`

Need to update the description to: s/<revision range>/<revision-range>/

Ack

quoted

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt

@@ -11,7 +11,7 @@ given by a list of patterns.
-'git sparse-checkout <subcommand> [options]'
+'git sparse-checkout <subcommand> [<options>...]'

The addition of `...` would make more sense if it was spelled
"option", but with it already being plural "options", I have trouble
understanding why `...` is added.

quoted

diff --git a/Documentation/git-stage.txt b/Documentation/git-stage.txt

@@ -9,7 +9,7 @@ git-stage - Add file contents to the staging area
-'git stage' args...
+'git stage' <arg>...

It's subjective, but I find plain `<args>` easier to interpret than
`<arg>...`. Does our documentation favor one form over the other, or
is there a random mix?

quoted

diff --git a/Documentation/git-web--browse.txt b/Documentation/git-web--browse.txt

@@ -8,7 +8,7 @@ git-web--browse - Git helper script to launch a web browser
-'git web{litdd}browse' [<options>] <url|file>...
+'git web{litdd}browse' [<options>] (<url>|<file>)...

Good.