Re: [PATCH 3/3] builtin/grep: allow implicit --no-index

From: Jeff King <peff@peff.net>
Date: Jan 11, 2016, 17:59 UTC
Message-ID: <20160111175919.GA11724@sigill.intra.peff.net>
In-Reply-To: <20160111174817.GC10612@hank>

On Mon, Jan 11, 2016 at 06:48:17PM +0100, Thomas Gummerer wrote:

Show 13 quoted lines

> > "We run 'git grep' in random places and relied on it to fail when
> > run in somewhere not under control of Git." feels so flawed at
> > multiple levels that I doubt it deserves to be kept working.  For
> > one thing, "git grep" is not the way to tell something is under
> > control of Git (rev-parse would be a better thing for scriptor to
> > use).  For another, how would such a script tell between "not a
> > git repository" and there was no hits?
> 
> I agree that scripts don't deserve to be kept working in that case.
> What about a user though who accidentally runs git grep outside of a
> repository, and is usually warned by git failing quickly, whereas with
> the changed behavior some time might go by until the user realizes the
> error.  Not sure if we want to support this use case or not?

Yeah, I don't think git would be _wrong_ here, but I could certainly see it being annoying. Several times a week I probably run `git grep` in my home directory, and after seeing its error, realize "oops, I meant to `cd git`".

Having it spew nonsense results, and/or appear to hang while it literally reads every file on my disk would be at least a minor annoyance.

But I don't think any kind of command-line flag would help that; I'm not going to start typing "git grep --use-index=never" for every invocation. I think the only sensible mitigation would be a config option, so that people who rarely use `--no-index` (and are OK with having to specify it) do not get punished by false positives.

I dunno. Maybe I would find the new behavior so useful I would be OK with the occasional false-positive. But when we make a release with the new behavior and somebody _does_ complain, it sure would be nice not to have to say "deal with it; it's the new behavior and there is no escape hatch".

-Peff