From: Jeff King <peff@peff.net>
Date: Thu, 13 Mar 2008 14:04:15 GMT
Subject: Re: [PATCH 10/16] add NO_EXTERNAL_GREP build option
Message-ID: <20080313140415.GB30348@coredump.intra.peff.net>
In-Reply-To: <47D9332C.4020904@viscovery.net>

On Thu, Mar 13, 2008 at 02:59:08PM +0100, Johannes Sixt wrote:

> > +	status = run_command_v_opt(argv, 0);
> ...
> > +	return status == 0 ? 1 : -1;
> 
> grep can return 0 (success, something found), 1 (nothing found), and other
> values for "real" failures like usage errors or crashes. This conditional
> throws the latter two into the same pot, which makes git-grep unable to
> distinguish "nothing found" from failure; cf. the call sites of
> flush_grep(), where want to set the flag 'hit'.

I noticed that, as well, while writing it. But if you look at
external_grep, it just lumps the two cases together anyway. It only ever
compares "0 < status", so I think the behavior should be identical.

Which isn't to say that the code couldn't be _improved_ by noting grep
failures and crashes, but I don't think it is any worse.

-Peff