Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add

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

 



On Thu, Dec 21, 2023 at 09:02:15AM -0800, Junio C Hamano wrote:

> > Yeah. I think it is in the same boat as the other two, in that I believe
> > that the KEEP_UNKNOWN_OPT flag is counter-productive and should just be
> > dropped.
> 
> If we dropped KEEP_UNKNOWN_OPT, however, the pattern that is
> currently accepted will stop working, e.g.,
> 
>  $ git sparse-checkout [add/set] --[no-]cone --foo bar
> 
> as we would barf with "--foo: unknown option", and the users who are
> used to this sloppy command line parsing we had for the past few
> years must now write "--end-of-options" before "--foo".  After all,
> the reason why the original authors of this code used KEEP_UNKNOWN
> is likely to deal with path patterns that begin with dashes.

Right, that is the "gotcha" I mentioned in my other email. Though that
is the way it has behaved historically, my argument is that users are
unreasonable to expect it to work:

  1. It is not consistent with the rest of Git commands.

  2. It is inconsistent with respect to existing options (and is an
     accident waiting to happen when new options are added).

So I'd consider it a bug-fix.

> The patch in the message that started this thread may not be
> correct, either, I am afraid.  For either of these:
> 
>  $ git sparse-checkout [add/set] --[no-]cone foo --end-of-options bar
>  $ git sparse-checkout [add/set] --[no-]cone --foo --end-of-options bar
> 
> we would see that "foo" (or "--foo") is not "--end-of-options", and
> we end up using three patterns "foo" (or "--foo"),
> "--end-of-options", and "bar", I suspect.  I wonder if we should
> notice the "foo" or "--foo" that were not understood and error out,
> instead.

Yes, I agree that "--foo --end-of-options" should barf. And of course
that happens naturally if you just let parse-options do its job by not
passing the KEEP_UNKNOWN_OPT flag. ;)

I'm not sure about "foo". We do allow out-of-order options/args, so
isn't it correct to keep it as a non-option argument?

> But after all, it is not absolutely necessary to notice and barf.
> The ONLY practical use of the "--end-of-options" mechanism is to
> allow us to write (this applies to any git subcommand):
> 
>  #!/bin/sh
>  git cmd --hard --coded --options --end-of-options "$@"
> 
> in scripts to protect the intended operation from mistaking the
> end-user input as options.  And with a script written carefully to
> do so, all the args that appear before "--end-of-options" would be
> recognizable by the command line parser.

Yes, but if you misspell "--otpions", it magically becomes a parameter
rather than having the command barf and complain. That is not the end of
the world, but it is unfriendly and inconsistent with the rest of Git.

-Peff




[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