On 13.01.13 23:38, Junio C Hamano wrote: > Jonathan Nieder <jrnieder@xxxxxxxxx> writes: > >> Hi, >> >> Torsten Bögershausen wrote: >> >>> - /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)'; >>> + /^\s*[^#]\s*which\s+[-a-zA-Z0-9]+$/ and err 'which is not portable (please use type)'; >> >> Hmm. Neither the old version nor the new one matches what seem to >> be typical uses of 'which', based on a quick code search: >> >> if which sl >/dev/null 2>&1 >> then >> sl -l >> ... >> fi >> >> or >> >> if test -x "$(which sl 2>/dev/null)" >> then >> sl -l >> ... >> fi > > Yes, these two misuses are what we want it to trigger on, so the > test is very easy to trigger and produce a false positive, but does > not trigger on what we really want to catch. > > That does not sound like a good benefit/cost ratio to me. > Thanks for comments, I think writing a regexp for which is difficult. What do we think about something like this for fishing for which: --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -644,6 +644,10 @@ yes () { : done } +which () { + echo >&2 "which is not portable (please use type)" + exit 1 +} This will happen in runtime, which might be good enough ? @Matt: >The "[^#]" appears to ensure that there's at least one character >before the which and that it's not a pound sign. Why is this done? This is simply wrong. -- 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