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

From: Jiang Xin <worldhello.net@gmail.com>
Date: Jul 9, 2021, 01:33 UTC
Message-ID: <CANYiYbGYzbMoU_2wb4duppASoYUjGLsJsr692Xe3GaVBOXUsBA@mail.gmail.com>
In-Reply-To: <YOcvaFL7+6qcIOUa@camp.crustytoothpaste.net>

brian m. carlson <sandals@crustytoothpaste.net> 于2021年7月9日周五 上午1:02写道：

Show 30 quoted lines

>
> On 2021-07-08 at 15:03:16, Jiang Xin wrote:
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > Options and revisions can be seperated by the option "--end-of-options"
> > by introducing commit 19e8789b23 (revision: allow --end-of-options to
> > end option parsing, 2019-08-06).  The following command will show
> > revisions which have changes on file "bar" on a branch named "--foo":
> >
> >     git rev-list --oneline --end-of-options --foo -- bar
> >
> > If we want to see revisions between two revisions (rev1 and rev2), we
> > can use the following command:
> >
> >     git rev-list --oneline --end-of-options rev1..rev2 --
> >
> > We know that "rev1..rev2" is a shorthand for "rev2 --not rev1", but
> > we can not use the longer expression with option "--not" after the
> > "--end-of-options" option.  This is because the parser will not consume
> > revision pseudo options after seeing "--end-of-option".
> >
> > Allow parsing revision pseudo options after "--end-of-options", the
> > following command is valid:
> >
> >     git rev-list --oneline --end-of-options rev2 --not rev2 --
>
> I don't think we want to do this.  The goal of --end-of-options is to
> prevent parsing all future items as options, so if someone specifies a
> revision starting with a dash, we don't end up with it being interpreted
> as an option.

New test case in t6000 covered this case. Branch "--output=yikes" which starts with a dash is used as revision after the option "--end-of-options", and it won't be interpreted as an option.

    test_expect_success 'parse pseudo option "--not" after "--end-of-options"' '
        cat >expect <<-EOF &&
        > three
        EOF
        git log --pretty="%m %s" --end-of-options \
                HEAD --not --output=yikes -- \
                two/three >actual &&
        test_cmp expect actual
    '

But for the original implementation, because pseudo revision options (--branches, --tags, --not, ..., etc) can not be used after the "--end-of-options" option, we have to put "--end-of-options" at the end of revisions, such as:

    git log --pretty="%m %s" rev1 --not rev2 rev3 rev4 \
            --end-of-options -- path/file

We can see from the above command, the option "--end-of-options" is immediately followed by a dashdash. That is very strange. DashDash is designed to separate pathspecs from args, and "--end-of-options" is designed to separate revisions from options. But because of the pseudo revision options, "--end-of-options" is meaningless for commands calling "setup_revisions()".

Yes, "--end-of-options" must be used if there is a revision which starts with dash, such as branch "--output=yikes" in t6000. That's even stranger, for we have to write command in the middle of revisions like this:

    git log --pretty="%m %s" rev1 --not rev2 rev3 \
            --end-of-options --output=yikes -- path/file

I know "rev1..rev2" and "rev2 ^rev1", but I prefer to use "rev1 --not rev2 rev3" instead of "rev1 ^rev2 ^rev3".