Hi,
On Sat, Dec 23, 2023 at 2:02 AM Jeff King <peff@xxxxxxxx> wrote:
>
> On Thu, Dec 21, 2023 at 02:04:56PM -0800, Junio C Hamano wrote:
>
> > Jeff King <peff@xxxxxxxx> writes:
> >
> > > 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.
> >
> > So the counter-proposal here is just to drop KEEP_UNKNOWN_OPT and
> > deliberately break them^W^W^Wrealign their expectations?
>
> Yes. :) But keep in mind we are un-breaking other people, like those who
> typo:
>
> git sparse-checkout --sikp-checks
>
> and don't see an error (instead, we make a garbage entry in the sparse
> checkout file).
I think you mean
git sparse-checkout set --sikp-checks
or
git sparse-checkout add --sikp-checks
(i.e. with either 'set' or 'add' in there)
Neither of these are currently an error, but I agree both definitely
ought to be. (Without the 'set' or 'add', you currently do get an
error, as we'd expect.)
> > I do not have much stake in sparse-checkout, so I am fine with that
> > direction. But I suspect other folks on the list would have users
> > of their own who would rather loudly complain to them if we broke
> > them ;-)
>
> Likewise, I have never used sparse-checkout myself, and I don't care
> _that_ much. My interest is mostly in having various Git commands behave
> consistently. This whole discussion started because the centralized
> --end-of-options fix interacted badly with this unusual behavior.
Well, I care quite a bit about sparse-checkout, and I would rather see
Peff's proposed fix, even if some users might have to tweak their
commands a little.
Of course, I'm not the only sparse-checkout user representative, but
Randall commented elsewhere in this thread that he used
sparse-checkout quite a bit, and he feels that "without the --,
--sikp-checks should result in an error." In other words, he is
basically agreeing that taking Peff's fix seems more important than
strict backward compatibility here. (His wording may not sound like
that at first glance, probably because of the 'set'/'add' omission in
the example command, but I think what he said actually amounts to
agreeing with this change.)
Finally, git-sparse-checkout(1) does say "THIS COMMAND IS
EXPERIMENTAL. ITS BEHAVIOR...WILL LIKELY CHANGE". I apologize for
being pulled from my intent to work on [*], which I think would allow
us to finally drop this warning (I'll still get back to it, one way or
another...), but I think this is a good case where we should take
advantage of the existing warning and simply fix the command.
Anyway, just my $0.02,
Elijah
[*] https://lore.kernel.org/git/pull.1367.v4.git.1667714666810.gitgitgadget@xxxxxxxxx/
