{"thread":{"id":"54776","subject":"`git grep` is too picky about option parsing","startedAt":"2020-12-06T22:24:29Z","lastAt":"2020-12-07T19:36:43Z","messageCount":5,"participants":["Jan Engelhardt","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"411566","messageId":"704q5rs6-63q1-sp78-9845-227oq8q42o8q@vanv.qr","threadId":"54776","inReplyTo":null,"subject":"`git grep` is too picky about option parsing","fromName":"Jan Engelhardt","fromEmail":"jengelh@inai.de","sentAt":"2020-12-06T22:12:43Z","receivedAt":"2020-12-06T22:24:29Z","isPatch":false,"sender":{"key":"jengelh@inai.de","avatar":"https://avatars.githubusercontent.com/u/8861948?v=4"},"body":"Version: 2.29.2\n\n\n-e, -i, -l and -n are all valid options for grep as well as git grep, \nyet git-grep refuses to operate if they appear in a specific order.\n\nObserved:\n\n$ git grep -e abc -lin\nerror: did you mean `--lin` (with two dashes)?\n\nExpected:\n\n$ git grep -e abc -lin\nsomefile\n"},{"id":"411607","messageId":"X85gMs1gPBNLff7f@coredump.intra.peff.net","threadId":"54776","inReplyTo":"704q5rs6-63q1-sp78-9845-227oq8q42o8q@vanv.qr","subject":"Re: `git grep` is too picky about option parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-12-07T17:02:42Z","receivedAt":"2020-12-07T17:03:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 06, 2020 at 11:12:43PM +0100, Jan Engelhardt wrote:\n\n> -e, -i, -l and -n are all valid options for grep as well as git grep,\n> yet git-grep refuses to operate if they appear in a specific order.\n> \n> Observed:\n> \n> $ git grep -e abc -lin\n> error: did you mean `--lin` (with two dashes)?\n> \n> Expected:\n> \n> $ git grep -e abc -lin\n> somefile\n\nHrm. This is falling afoul of the typo-detection added by 3a9f0f41db\n(parse-options: catch likely typo in presense of aggregated options.,\n2008-01-26), which wonders if you meant \"--line-number\" (we allow long\noptions to be prefixes, so --lin is a synonym; the double irony is that\n\"-n\" is also a synonym here). A few other simplified examples:\n\n  # works; not a prefix of a long option\n  git grep -lni foo\n\n  # does not work; prefix of --line-number\n  git grep -lin foo\n\n  # works; we require 3 characters before we complain\n  git grep -li foo\n\nThe problem is that it gives bundled short options a lower precedence\nthan detecting possible typos against long options. My instinct is to\nsay that this is wrong. We should allow valid things to work, and only\nadd error heuristics if the user's request is nonsense (i.e., if one of\nthe bundled options is not a valid one). But that actually contradicts\nthe original example given in 3a9f0f41db! There it was trying to make:\n\n  git commit -amend\n\nan error. But that's a set of valid options, the same as:\n\n  git commit -a -m end\n\nSo we'd be losing that protection. Another option would be to make the\ntypo-checker a little more picky:\n\n  - require more than 3 characters; this is just punting off the\n    problem, though. Doing \"-line foo\" is valid. So is \"-linefoo\", for\n    that matter, though that one would do what we want since it stops\n    being a prefix.\n\n  - be more aggressive about how much of a long option we match in the\n    prefix (at least for the typo checker). \"lin\" is an awfully small\n    part of \"line-number\". People may plausibly use \"--lin\" or \"--line\"\n    as a shortcut, but I'm not sure that merits blocking the valid\n    \"-lin\" for the typo-checker.\n\nEither of those would let \"-amend\" continue to be an error, but fix\n\"-lin\".\n\n-Peff\n"},{"id":"411618","messageId":"xmqqa6upbgil.fsf@gitster.c.googlers.com","threadId":"54776","inReplyTo":"X85gMs1gPBNLff7f@coredump.intra.peff.net","subject":"Re: `git grep` is too picky about option parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T18:46:58Z","receivedAt":"2020-12-07T18:47:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The problem is that it gives bundled short options a lower precedence\n> than detecting possible typos against long options. My instinct is to\n> say that this is wrong. We should allow valid things to work, and only\n> add error heuristics if the user's request is nonsense (i.e., if one of\n> the bundled options is not a valid one).\n\nThanks, I was coming to the same conclusion.\n\n> But that actually contradicts\n> the original example given in 3a9f0f41db! There it was trying to make:\n>\n>   git commit -amend\n>\n> an error. But that's a set of valid options, the same as:\n>\n>   git commit -a -m end\n>\n> So we'd be losing that protection. Another option would be to make the\n> typo-checker a little more picky:\n>\n>   - require more than 3 characters; this is just punting off the\n>     problem, though. Doing \"-line foo\" is valid. So is \"-linefoo\", for\n>     that matter, though that one would do what we want since it stops\n>     being a prefix.\n>\n>   - be more aggressive about how much of a long option we match in the\n>     prefix (at least for the typo checker). \"lin\" is an awfully small\n>     part of \"line-number\". People may plausibly use \"--lin\" or \"--line\"\n>     as a shortcut, but I'm not sure that merits blocking the valid\n>     \"-lin\" for the typo-checker.\n>\n> Either of those would let \"-amend\" continue to be an error, but fix\n> \"-lin\".\n\nI am wondering if a rule like \"you cannot concatenate a short option\nthat takes argument with other short options\" work.  The problem\nwith \"-a -m end\" is really that the 'm' takes arbitrary end-user\ninput.  So \"commit -ave\" would be fine, but \"commit -ame\" would not\nbe.  This would make both \"-line foo\" and \"--linefoo\" consistently\ninvalid, but \"-lin -e foo\" is still OK and make the rule easier to\nexplain.\n\nThen we can probably lift the \"more than 3 characters\" heuristics,\nwhich may be a good thing independently.\n\n\n"},{"id":"411620","messageId":"X856rSHziQcmr/zX@coredump.intra.peff.net","threadId":"54776","inReplyTo":"xmqqa6upbgil.fsf@gitster.c.googlers.com","subject":"Re: `git grep` is too picky about option parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-12-07T18:55:41Z","receivedAt":"2020-12-07T18:56:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 07, 2020 at 10:46:58AM -0800, Junio C Hamano wrote:\n\n> > Either of those would let \"-amend\" continue to be an error, but fix\n> > \"-lin\".\n> \n> I am wondering if a rule like \"you cannot concatenate a short option\n> that takes argument with other short options\" work.  The problem\n> with \"-a -m end\" is really that the 'm' takes arbitrary end-user\n> input.  So \"commit -ave\" would be fine, but \"commit -ame\" would not\n> be.  This would make both \"-line foo\" and \"--linefoo\" consistently\n> invalid, but \"-lin -e foo\" is still OK and make the rule easier to\n> explain.\n\nPersonally, I find \"-linefoo\" totally unreadable (and in general I find\nthe \"stuck\" form of short options with a string to be pretty ugly,\nthough I understand it is the recommended form to handle optional\narguments). But \"-line foo\" is not so horrible IMHO, and I think it\nwould be sad to lose it. (I don't use it with grep, but my standard perl\ninvocation is \"perl -lpe 'some script'\"; another common one is \"tar xvf\nfoo.tar\").\n\nAnd it works now (obviously not in the case we're discussing, but in\nother cases that don't run afoul of the typo fix), so I think people\nwould see that as a regression.\n\n-Peff\n"},{"id":"411639","messageId":"xmqqtusx9zpn.fsf@gitster.c.googlers.com","threadId":"54776","inReplyTo":"X856rSHziQcmr/zX@coredump.intra.peff.net","subject":"Re: `git grep` is too picky about option parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T19:35:16Z","receivedAt":"2020-12-07T19:36:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> would be sad to lose it. (I don't use it with grep, but my standard perl\n> invocation is \"perl -lpe 'some script'\"; another common one is \"tar xvf\n> foo.tar\").\n\nHeh, \"tar\" is a different breed, isn't it?  It can have a clump of\nsingle letter options among which more than one take parameter.\n\n> And it works now (obviously not in the case we're discussing, but in\n> other cases that don't run afoul of the typo fix), so I think people\n> would see that as a regression.\n\nSure.\n"}]}