Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 13 2022, Junio C Hamano wrote:

> Teng Long <dyroneteng@xxxxxxxxx> writes:
>
>> +test_must_fail 'Exchange the order of "--heads" and <remote>' '
>> +    git --version &&
>> +    git init "test.git" &&
>> +    test_commit -C "test.git" one &&
>> +    git -C "test.git" ls-remote --heads ./. > result.1 &&
>> +    git -C "test.git" ls-remote ./. --heads > result.2 &&
>
> I would say that this is working exactly as designed.  As with the
> unix tradition, after the command name, first come options
> (e.g. "--heads", "-v", etc. that begin with a dash or two dashes),
> then arguments like "origin", "master", "." that are not dashed
> options/flags.
>
> Then among the arguments, we generally take revs first and then
> pathspecs.  "git help cli" explicitly mentions this, because it is
> specific to "git" command suite, but it does not mention "dashed
> options/flags first and then args", primarily because, at least back
> when the documentation was written, this was taken as granted, iow,
> those who wrote the "gitcli" documentation thought it was a common
> knowledge among users that did not need to be spelled out.
>
> Apparently, it is not a common knowledge at least for you (and
> probably others).  Perhaps we should add a paragraph to the cli help
> and explicitly mention "options first and then args", before we go
> on to say "among args, revs first and then pathspecs".

I don't think this summary is accurate.

We have multiple commands that are in GNU-fashion loose about whether
you provide options first before no-option args, or after. E.g. we
accept both of:

    git push --dry-run <remote> <ref>

And:

    git push <remote> <ref> --dry-run

The "tradition" you're referring to accurately summarizes how POSIX
specifies that things should work.

But when GNU came around its option parser was generally happy to accept
options and args in either order. E.g. these both work with GNU
coreutils, but the latter will fail on FreeBSD and various other
capital-U UNIX-es:

    touch foo; rm -v foo
    touch foo; rm foo -v

The relevant documentation being
https://www.gnu.org/prep/standards/standards.html#Standards-for-Command-Line-Interfaces;
specifically:

    "Note that the GNU version of getopt will normally permit options
    anywhere among the arguments unless the special argument ‘--’ is
    used. This is not what POSIX specifies; it is a GNU extension."

Now, as to git's behavior we mostly follow the GNU style. The default
behavior of parse_options() is to accept the looser GNU style. In this
case we're explicitly passing the PARSE_OPT_STOP_AT_NON_OPTION flag.

But why is that?

Most things that do that are doing so because we're parsing things in
two passes, i.e. for commands that have sub-commands, e.g.:

    git commit-graph [opts] [subcommand] [subcommand-opts]

In those cases we use PARSE_OPT_STOP_AT_NON_OPTION on our first pass, so
that we'll stop at "subcommand", then we'll generally continue the
parsing as of "[subcommand]" without that flag.

The general exception to that rule, as you note, being things that take
pathspecs, although for some of those we'll apply other custom
disambiguation.

In this case a look at ba5f28bf79e (ls-remote: use parse-options api,
2016-01-19) will show that it was faithfully converted from an ad-hoc
custom option parser using the "BSD-like" option parsing discussed
above.

I really don't see why we wouldn't take this patch given that the
command has one parse_options() invocation (so no subcommand
complexity), and doesn't accept pathspecs etc.

We should aim to make the command line UX consistent when possible, not
slavishly bug-for-bug follow whatever the behavior of the
pre-parse_options() code was.

We can't do that in some cases, but this case seems like a pretty clear
case where simply removing the PARSE_OPT_STOP_AT_NON_OPTION flag would
be an improvement. It's not ambiguous, and makes invocations that
currently (and inconsistently v.s. our usual behavior) error out work.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux