From: Martin Ågren <martin.agren@gmail.com>
Date: Wed, 27 Oct 2021 18:56:07 GMT
Subject: Re: [PATCH] doc: fix grammar rules in commands'syntax
Message-ID: <CAN0heSpRZy3+jyc09NEj4NJk4zN4X_RyVk33F5c6tyUE2qMGzQ@mail.gmail.com>
In-Reply-To: <pull.1066.git.1635261072531.gitgitgadget@gmail.com>

On Tue, 26 Oct 2021 at 21:35, Jean-Noël Avila via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> --- a/Documentation/git-archimport.txt
> +++ b/Documentation/git-archimport.txt
> @@ -9,8 +9,8 @@ git-archimport - Import a GNU Arch repository into Git
>  SYNOPSIS
>  --------
>  [verse]
> -'git archimport' [-h] [-v] [-o] [-a] [-f] [-T] [-D depth] [-t tempdir]
> -               <archive/branch>[:<git-branch>] ...
> +'git archimport' [-h] [-v] [-o] [-a] [-f] [-T] [-D <depth>] [-t <tempdir>]
> +              <archive>/<branch>[:<git-branch>]...

Your rewrite makes it seem like one would write, e.g., "myarch/master"
with a literal slash, whereas my initial thought was that the original
tried to express something like "(<archive> | <branch>)". But I have
zero experience with "GNU Arch" and git-archimport, so I can't really
tell whether your rewrite is for the better or not. :-)

In any case, this document goes on to write "<archive/branch>" several
times. Supposedly, they would all want to be changed as well. There's
also an instance of "Archive/branch identifier ..." to maybe look into.

> --- 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
>  SYNOPSIS
>  --------
>  [verse]
> -'git cvsimport' [-o <branch-for-HEAD>] [-h] [-v] [-d <CVSROOT>]
> +'git cvsimport' [-o <branch-for-HEAD>] [-h] [-v] [-d <cvsroot>]

> -<CVS_module>::
> +<CVS-module>::
>         The CVS module you want to import. Relative to <CVSROOT>.

Here's another "<CVSROOT>".

> --- a/Documentation/git-http-push.txt
> +++ b/Documentation/git-http-push.txt
> @@ -63,16 +63,15 @@ of such patterns separated by a colon ":" (this means that a ref name

> -Each pattern pair consists of the source side (before the colon)
> -and the destination side (after the colon).  The ref to be
> -pushed is determined by finding a match that matches the source
> -side, and where it is pushed is determined by using the
> -destination side.
> +Each pattern pair '<src>:<dst>' consists of the source side (before
> +the colon) and the destination side (after the colon).  The ref to be
> +pushed is determined by finding a match that matches the source side,
> +and where it is pushed is determined by using the destination side.

This looks like the insertion of "'<src>:<dst>' early on, where the rest
of the changes are just follow-on line-wrapping.

I wonder if this patch could benefit from being broken into smaller
pieces. Maybe a few preliminaries like "change <foo|bar|baz> to
(foo|bar|baz)" and the like, then even if the final patch is "large", it
will not be *as large*? But there are clearly sub-topics here, such as
"change <some_arg> to <some-arg>" and "change arg to <arg>". Or maybe
this doesn't make sense as an approach to cutting this patch into
smaller pieces, but I thought I'd mention it.

> - - It is an error if <src> does not match exactly one of the
> + - It is an error if '<src>' does not match exactly one of the
>     local refs.
>
> - - If <dst> does not match any remote ref, either
> + - If '<dst>' does not match any remote ref, either

I believe these match Junio's preference, so ok. But again, this looks
like it could go in a separate patch from a lot of these other changes
as a way of keeping to somewhat focused changes.

> -               (--[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)...]

Sort of cute how this saves on repeating the "--" by pulling it out.
Anyway, nothing new in your patch. :-)

Thanks for unifying these things.

Martin