git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 6/7] grep: avoid resolving revision names in --no-index case

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Feb 14, 2017, 18:19 UTC
Message-ID
<82212eaa-76d2-3357-8e06-5e4d56028c2e@google.com>
In-Reply-To
<20170214180453.tpze2hdv3eytxfju@sigill.intra.peff.net>
On 02/14/2017 10:04 AM, Jeff King wrote:
Show 31 quoted lines
> On Tue, Feb 14, 2017 at 08:53:04AM -0800, Jonathan Tan wrote:
>
>> On 02/13/2017 10:07 PM, Jeff King wrote:
>>> diff --git a/builtin/grep.c b/builtin/grep.c
>>> index e83b33bda..c4c632594 100644
>>> --- a/builtin/grep.c
>>> +++ b/builtin/grep.c
>>> @@ -1176,6 +1176,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>>>  			break;
>>>  		}
>>>
>>> +		if (!use_index) {
>>> +			if (seen_dashdash)
>>> +				die(_("--no-index cannot be used with revs"));
>>
>> There is a subsequent check that prints "--no-index or --untracked cannot be
>> used with revs." - maybe we should just expand this part to incorporate that
>> case. (That is, write `if (!use_index || untracked)` instead of `if
>> (!use_index)`.) This also allows us to preserve the error message, which
>> might be useful for someone using a translated version of Git.
>
> I wasn't sure if we wanted to treat "untracked" in the same way.
> Certainly we can catch the error here for the seen_dashdash case, but is
> it also the case that:
>
>   echo content >master
>   git grep --untracked pattern master
>
> should treat "master" as a path?
>
> -Peff
It is already the case that it cannot be treated as a rev:
   $ git grep --untracked pattern master
   fatal: --no-index or --untracked cannot be used with revs.

So I think it would be better if it was treated as a path, for consistency with --no-index. I'm OK either way, though.

Previous: Jeff KingNext: Jeff King
Message 17 of 22 in “grep: do not unnecessarily query repo for "--"”
  1. grep: do not unnecessarily query repo for "--"Jonathan Tan, Feb 14, 2017
  2. Jeff KingFeb 14, 2017
  3. 0/7 grep rev/path parsing fixesJeff King, Feb 14, 2017
  4. 1/7 grep: move thread initialization a little lowerJeff King, Feb 14, 2017
  5. Brandon WilliamsFeb 14, 2017
  6. 2/7 grep: do not unnecessarily query repo for "--"Jeff King, Feb 14, 2017
  7. 3/7 t7810: make "--no-index --" test more robustJeff King, Feb 14, 2017
  8. 4/7 grep: re-order rev-parsing loopJeff King, Feb 14, 2017
  9. Brandon WilliamsFeb 14, 2017
  10. 5/7 grep: fix "--" rev/pathspec disambiguationJeff King, Feb 14, 2017
  11. Brandon WilliamsFeb 14, 2017
  12. Jeff KingFeb 14, 2017
  13. Junio C HamanoFeb 14, 2017
  14. 6/7 grep: avoid resolving revision names in --no-index caseJeff King, Feb 14, 2017
  15. Jonathan TanFeb 14, 2017
  16. Jeff KingFeb 14, 2017
  17. Jonathan TanFeb 14, 2017
  18. 8/7 grep: treat revs the same for --untracked as for --no-indexJeff King, Feb 14, 2017
  19. Junio C HamanoFeb 14, 2017
  20. 7/7 grep: do not diagnose misspelt revs with --no-indexJeff King, Feb 14, 2017
  21. Jeff KingFeb 14, 2017
  22. Jonathan TanFeb 14, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.