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

[Thomas Gummerer <t.gummerer@gmail.com>]

On 01/11, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > On 01/11, Duy Nguyen wrote:
> >> On Sun, Jan 10, 2016 at 9:19 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> >> > Currently when git grep is used outside of a git repository without the
> >> > --no-index option git simply dies.  For convenience, implicitly make git
> >> > grep behave like git grep --no-index when it is called outside of a git
> >> > repository.
> >>
> >> Should we have a line about this behavior in git-grep.txt, maybe the
> >> description section?
> >
> > Yes good point, the behavior change should definitely be documented.
> >
> >> I wonder if anybody wants the old behavior (e.g.
> >> non-zero exit code when running outside a repo). If there is such a
> >> case (*), we may need an option to revert it back (--no-no-index seems
> >> ridiculous, maybe --use-index). The safest way though, is introduce a
> >> new option like --use-index=<always|optional|never> then you can make
> >> an grep alias with --use-index=optional.
> >
> > You're right.  I couldn't think of a reason why someone would rely on
> > the old behavior, but maybe I missed something.  I like the idea of
> > introducing the --use-index=... option.
>
> I don't like that, though ;-)
>
> "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?

> So I do agree that automatic fallback needs to be documented and
> advertised as a feature (or even a bugfix), I do not think we want
> to add knobs to keep such a broken script working.
>
> > How should we handle priority between --no-index and --use-index,
> > should we just give --no-index priority if it is set and ignore the
> > new --use-index option, or is there some other way?
> >
> >> (*) I've been hitting really weird real-world use cases so I'm a bit paranoid..
> >> --
> >> Duy

--
Thomas Gummerer