git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jan Engelhardt <jengelh@inai.de>
Cc: git@vger.kernel.org
Subject: Re: `git grep` is too picky about option parsing
Date: Mon, 7 Dec 2020 12:02:42 -0500	[thread overview]
Message-ID: <X85gMs1gPBNLff7f@coredump.intra.peff.net> (raw)
In-Reply-To: <704q5rs6-63q1-sp78-9845-227oq8q42o8q@vanv.qr>

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

  reply	other threads:[~2020-12-07 17:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-06 22:12 `git grep` is too picky about option parsing Jan Engelhardt
2020-12-07 17:02 ` Jeff King [this message]
2020-12-07 18:46   ` Junio C Hamano
2020-12-07 18:55     ` Jeff King
2020-12-07 19:35       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=X85gMs1gPBNLff7f@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jengelh@inai.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).