Jeff King <peff@xxxxxxxx> writes:
>> git-grep currently throws an error when you combine the -F and -i
>> flags. This isn't in line with how GNU grep handles it. This patch
>> allows the simultaneous use of those flags.
>
> I don't see a reason not to allow this combination if our grep
> implementation supports it. My only reservation would be that we
> sometimes call out to an external grep, and non-GNU grep might barf on
> this. But I think that is OK, as the user should get a sane error from
> the external grep.
I think that is OK but not for the reason you stated. The user should
never get such an error message.
The reason why I think this is OK is because the builder can choose to use
NO_EXTERNAL_GREP if the external grep does not allow this combination.
We need to update comments on the Makefile if we are going to take this
patch. Currently the description suggests three possible reasons you
might want to choose NO_EXTERNAL_GREP. "lacks grep" is obvious, "slower"
is not about correctness, but "is broken" is way underspecified, and this
patch adds one more reason to label your "grep" as broken.
Currently, SunOS and IRIX/IRIX64 are the only ones that specify
NO_EXTERNAL_GREP, and I suspect both are defined due to "is broken", but
we do not tell the builder in what way they are considered broken, iow,
what features we expect from the platform "grep", so somebody coming up
with a new port would be at loss.
I suspect 01ae841 (SunOS grep does not understand -C<n> nor -e, 2009-07-23)
would be a good starting point. Something like this...
# Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
# your external grep (e.g., if your system lacks grep, if its grep
# does not support necessary features, or spawning external process is
# slower than built-in grep git has). To be usable, your grep must
# support -C<n> (show n lines of context), -e <pattern> (primarily
# used to quote a pattern that begins with a dash), and use of -F and
# -i at the same time. Otherwise define this.
But I didn't try hard to find out what _else_ we are depending on.
> Tests? They help prove to us that your feature works, and also prevent
> us from accidentally breaking your feature in the future.
Yeah, thanks. The latter is the primary purpose of test.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html