On Fri, Nov 06, 2009 at 02:00:11AM -0800, Junio C Hamano wrote: > > 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. Yes, I think that would be a sane thing to do (and I suspect anyone using non-GNU grep is probably already doing it). But what I meant more was that the _transition_ should be fine. If we start shipping with this patch but people haven't updated their build configuration, it is not going to break horribly; it should just produce an error, which is what it is doing now. > # 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. It is not really _us_ depending on it. It is "things the user wants to do that _we_ support, but that their grep might not." So I don't think there is much point in enumerating features. If their system grep doesn't handle options that they want to use, then it won't work for them. If they don't use them, then they will be fine. Though "-e" might be the exception, as I think we might use it unconditionally. But something like "-F -i" really depends on whether the user wants to use it. So I am fine with the text above, but I wouldn't worry too hard about trying to come up with an exhaustive feature list. -Peff -- 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