From: Jonathan Tan <jonathantanmy@google.com>
Date: Tue, 14 Feb 2017 18:19:56 GMT
Subject: Re: [PATCH 6/7] grep: avoid resolving revision names in --no-index case
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:
> 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.