Re: [PATCH] sparse-checkout: be consistent with end of options markers

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > 93851746 (parse-options: decouple "--end-of-options" and "--",
> > 2023-12-06) updated the world order to make callers of parse-options
> > that set PARSE_OPT_KEEP_UNKNOWN_OPT responsible for deciding what to
> > do with "--end-of-options" they may see after parse_options() returns.
> >
> > This made a previous bug in sparse-checkout more visible; namely,
> > that
> >
> >   git sparse-checkout [add|set] --[no-]cone --end-of-options ...
> >
> > would simply treat "--end-of-options" as one of the paths to include in
> > the sparse-checkout.  But this was already problematic before; namely,
> >
> >   git sparse-checkout [add|set| --[no-]cone --sikp-checks ...
> >
> > would not give an error on the mis-typed "--skip-checks" but instead
> > simply treat "--sikp-checks" as a path or pattern to include in the
> > sparse-checkout, which is highly unfriendly.
> >
> > This behavior begain when the command was converted to parse-options in
> > 7bffca95ea (sparse-checkout: add '--stdin' option to set subcommand,
> > 2019-11-21).  Back then it was just called KEEP_UNKNOWN. Later it was
> > renamed to KEEP_UNKNOWN_OPT in 99d86d60e5 (parse-options:
> > PARSE_OPT_KEEP_UNKNOWN only applies to --options, 2022-08-19) to clarify
> > that it was only about dashed options; we always keep non-option
> > arguments.  Looking at that original patch, both Peff and I think that
> > the author was simply confused about the mis-named option, and really
> > just wanted to keep the non-option arguments.  We never should have used
> > the flag all along (and the other cases were cargo-culted within the
> > file).
> >
> > Remove the erroneous PARSE_OPT_KEEP_UNKNOWN_OPT flag now to fix this
> > bug.  Note that this does mean that anyone who might have been using
> >
> >   git sparse-checkout [add|set] [--[no-]cone] --foo --bar
> >
> > to request paths or patterns '--foo' and '--bar' will now have to use
> >
> >   git sparse-checkout [add|set] [--[no-]cone] -- --foo --bar
> >
> > That makes sparse-checkout more consistent with other git commands,
> > provides users much friendlier error messages and behavior, and is
> > consistent with the all-capps warning in git-sparse-checkout.txt that
> > this command "is experimental...its behavior...will likely change".  :-)
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
>
> Nicely described.
>
> Apparently we do not have an existing test to cover the case of
> "misspelt options becoming a pattern" that this bugfix corrects;
> otherwise we would have a s/failure/success/ to update such an older
> expectation, and have to wonder if the existing behaviour is
> intentional.  Since there is no such test, we can feel a bit safer
> in "fixing" this odd behaviour.
>
> Also, this corrects "--end-of-options" that becomes one of the
> patterns.  Such a bug was not detected in our test suite.
>
> So we should at least have two new tests.
>
>  (1) Pass "--foo" without the end of options marker and make sure we
>      error out, instead of making it as one of the patterns.
>
>  (2) Pass "--end-of-options foo" and make sure the command succeeds,
>      and "--end-of-options" does not become one of the patterns.
>
> Thanks.