Re: [PATCH] revision: allow pseudo options after --end-of-options

[Jiang Xin <worldhello.net@gmail.com>]

Jeff King <peff@peff.net> 于2021年7月13日周二 上午1:54写道：
> On Sat, Jul 10, 2021 at 09:54:16PM +0000, brian m. carlson wrote:
> > > I know "rev1..rev2" and "rev2 ^rev1", but I prefer to use "rev1 --not
> > > rev2 rev3" instead of "rev1 ^rev2 ^rev3".
> >
> > I don't think a personal preference is a good reason to change this.
>
> I do think it rises slightly above personal preference. It's potentially
> making things much easier for the caller if they can ferry along:
>
>   tip=$1; shift
>   git rev-list --end-of-options "$1" --not "$@"
>
> instead of:
>
>   tip=$1; shift
>   # whoops, whitespace splitting is wrong here! Real programming
>   # languages make this easier, of course.
>   git rev-list --end-of-options "$1" $(for i in "$@"; do echo "^$i"; done)
>
> Though in my experience it is usually a static "--not --all" or "--not
> --branches --tags" or similar in such a function. I don't think I've
> ever seen a case quite like the code above in practice.

Last week, I made a study on how gitlab wrap and execute a git
command. I saw the following code [1]:

    if c.supportsEndOfOptions() {
        commandArgs = append(commandArgs, "--end-of-options")
    }
    if len(postSepArgs) > 0 {
        commandArgs = append(commandArgs, "--")
    }

I was surprised to see the options "--end-of-options" and "--" used
next to each other, and the DashDash option ("--") is not mandatory. I
want to make some changes on it, but when I try to construct a git
command like this:

    git.SubCmd{
        Name: "log",
        Flags: []git.Option{
            git.Flag{
                    Name: "--stat"
            },
            git.ValueFlag{
                    Name: "--pretty",
                    Value: "%m %s",
            },
            git.Flag{
                    Name: "--no-decorate",
            },
        },
        Args: []string {
            "topic1",
            "--not",
            "main",
            "release",
        },
        PostSepArgs: []string {
            "src/hello.c",
            "doc",
        },
    }

The generated git command will be:

    git log --stat --pretty="%m %s" --no-decorate \
        topic1 --not main release \
        --end-of-options \
        -- \
        src/hello.c doc

It works. But if I move the "--end-of-options" before the revisions like this:

    git log --stat --pretty="%m %s" --no-decorate \
        --end-of-options \
       topic1 --not main release \
        -- \
        src/hello.c doc

The generated command failed to execute with error: unknown revision "--not".

It's reasonable for gitlab to construct git commands using mainly three fields:
1. Flags: for options like "--option", or "--key value".
2. Args: for args like revisions.
3. PostSepArgs: for pathspecs.

And if the command supports these options, it's better to add
"--end-of-options" between 1 and 2, and add "--" between 2 and 3.

If we can handle revision pseudo opts as pseudo revisions instead of
options as in this patch, the only disadvantage is that we cannot
handle branches whose names conflict with well-known options such as
"--not" and "--all". But users can input full branch names, such as
"refs/heads/--not", "refs/heads/--all".

Maybe I should keep this patch as a local enhancement for git.

1. https://gitlab.com/gitlab-org/gitaly/-/blob/v14.0.5/internal/git/command_description.go#L320-326