Thread (2 messages) 2 messages, 2 authors, 2023-12-21

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

From: Jeff King <hidden>
Date: 2023-12-21 21:45:51

On Thu, Dec 21, 2023 at 09:02:15AM -0800, Junio C Hamano wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help