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

From: Jiang Xin <worldhello.net@gmail.com>
Date: Jul 13, 2021, 08:57 UTC
Message-ID: <CANYiYbF3dcOzZ5gqbYkstxv+WRxZwnoxoNP28A6px44YD98k1Q@mail.gmail.com>
In-Reply-To: <YOyByjmGu1oDXK4X@coredump.intra.peff.net>

Jeff King <peff@peff.net> 于2021年7月13日周二 上午1:54写道：

Show 22 quoted lines

> 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