`git grep` is too picky about option parsing

`git grep` is too picky about option parsing

[Jan Engelhardt]

Version: 2.29.2


-e, -i, -l and -n are all valid options for grep as well as git grep, 
yet git-grep refuses to operate if they appear in a specific order.

Observed:

$ git grep -e abc -lin
error: did you mean `--lin` (with two dashes)?

Expected:

$ git grep -e abc -lin
somefile

Re: `git grep` is too picky about option parsing

[Jeff King]

On Sun, Dec 06, 2020 at 11:12:43PM +0100, Jan Engelhardt wrote:

> -e, -i, -l and -n are all valid options for grep as well as git grep,
> yet git-grep refuses to operate if they appear in a specific order.
> 
> Observed:
> 
> $ git grep -e abc -lin
> error: did you mean `--lin` (with two dashes)?
> 
> Expected:
> 
> $ git grep -e abc -lin
> somefile

Hrm. This is falling afoul of the typo-detection added by 3a9f0f41db
(parse-options: catch likely typo in presense of aggregated options.,
2008-01-26), which wonders if you meant "--line-number" (we allow long
options to be prefixes, so --lin is a synonym; the double irony is that
"-n" is also a synonym here). A few other simplified examples:

  # works; not a prefix of a long option
  git grep -lni foo

  # does not work; prefix of --line-number
  git grep -lin foo

  # works; we require 3 characters before we complain
  git grep -li foo

The problem is that it gives bundled short options a lower precedence
than detecting possible typos against long options. My instinct is to
say that this is wrong. We should allow valid things to work, and only
add error heuristics if the user's request is nonsense (i.e., if one of
the bundled options is not a valid one). But that actually contradicts
the original example given in 3a9f0f41db! There it was trying to make:

  git commit -amend

an error. But that's a set of valid options, the same as:

  git commit -a -m end

So we'd be losing that protection. Another option would be to make the
typo-checker a little more picky:

  - require more than 3 characters; this is just punting off the
    problem, though. Doing "-line foo" is valid. So is "-linefoo", for
    that matter, though that one would do what we want since it stops
    being a prefix.

  - be more aggressive about how much of a long option we match in the
    prefix (at least for the typo checker). "lin" is an awfully small
    part of "line-number". People may plausibly use "--lin" or "--line"
    as a shortcut, but I'm not sure that merits blocking the valid
    "-lin" for the typo-checker.

Either of those would let "-amend" continue to be an error, but fix
"-lin".

-Peff

Re: `git grep` is too picky about option parsing

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> The problem is that it gives bundled short options a lower precedence
> than detecting possible typos against long options. My instinct is to
> say that this is wrong. We should allow valid things to work, and only
> add error heuristics if the user's request is nonsense (i.e., if one of
> the bundled options is not a valid one).

Thanks, I was coming to the same conclusion.

> But that actually contradicts
> the original example given in 3a9f0f41db! There it was trying to make:
>
>   git commit -amend
>
> an error. But that's a set of valid options, the same as:
>
>   git commit -a -m end
>
> So we'd be losing that protection. Another option would be to make the
> typo-checker a little more picky:
>
>   - require more than 3 characters; this is just punting off the
>     problem, though. Doing "-line foo" is valid. So is "-linefoo", for
>     that matter, though that one would do what we want since it stops
>     being a prefix.
>
>   - be more aggressive about how much of a long option we match in the
>     prefix (at least for the typo checker). "lin" is an awfully small
>     part of "line-number". People may plausibly use "--lin" or "--line"
>     as a shortcut, but I'm not sure that merits blocking the valid
>     "-lin" for the typo-checker.
>
> Either of those would let "-amend" continue to be an error, but fix
> "-lin".

I am wondering if a rule like "you cannot concatenate a short option
that takes argument with other short options" work.  The problem
with "-a -m end" is really that the 'm' takes arbitrary end-user
input.  So "commit -ave" would be fine, but "commit -ame" would not
be.  This would make both "-line foo" and "--linefoo" consistently
invalid, but "-lin -e foo" is still OK and make the rule easier to
explain.

Then we can probably lift the "more than 3 characters" heuristics,
which may be a good thing independently.

Re: `git grep` is too picky about option parsing

[Jeff King]

On Mon, Dec 07, 2020 at 10:46:58AM -0800, Junio C Hamano wrote:

> > Either of those would let "-amend" continue to be an error, but fix
> > "-lin".
> 
> I am wondering if a rule like "you cannot concatenate a short option
> that takes argument with other short options" work.  The problem
> with "-a -m end" is really that the 'm' takes arbitrary end-user
> input.  So "commit -ave" would be fine, but "commit -ame" would not
> be.  This would make both "-line foo" and "--linefoo" consistently
> invalid, but "-lin -e foo" is still OK and make the rule easier to
> explain.

Personally, I find "-linefoo" totally unreadable (and in general I find
the "stuck" form of short options with a string to be pretty ugly,
though I understand it is the recommended form to handle optional
arguments). But "-line foo" is not so horrible IMHO, and I think it
would be sad to lose it. (I don't use it with grep, but my standard perl
invocation is "perl -lpe 'some script'"; another common one is "tar xvf
foo.tar").

And it works now (obviously not in the case we're discussing, but in
other cases that don't run afoul of the typo fix), so I think people
would see that as a regression.

-Peff

Re: `git grep` is too picky about option parsing

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> would be sad to lose it. (I don't use it with grep, but my standard perl
> invocation is "perl -lpe 'some script'"; another common one is "tar xvf
> foo.tar").

Heh, "tar" is a different breed, isn't it?  It can have a clump of
single letter options among which more than one take parameter.

> And it works now (obviously not in the case we're discussing, but in
> other cases that don't run afoul of the typo fix), so I think people
> would see that as a regression.

Sure.