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

> 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?

> 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