On Fri, May 12, 2017 at 7:06 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> Add a helper function to make the tests which check for patterns with
>> \0 in them more succinct. Right now this isn't a big win, but
>> subsequent commits will add a lot more of these tests.
>>
>> The helper is based on the match() function in t3070-wildmatch.sh.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
>> ---
>> t/t7008-grep-binary.sh | 58 +++++++++++++++++++++++++-------------------------
>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>
>> diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
>> index 9c9c378119..6c1952eafa 100755
>> --- a/t/t7008-grep-binary.sh
>> +++ b/t/t7008-grep-binary.sh
>> @@ -4,6 +4,29 @@ test_description='git grep in binary files'
>>
>> . ./test-lib.sh
>>
>> +nul_match() {
>
> Micronit: "nul_match () {"
>
>> + status=$1
>> + flags=$2
>> + pattern=$3
>> + pattern_human=$(echo $pattern | sed 's/Q/<NUL>/g')
>
> Double quote around "$pattern"?
>
>> +
>> + if test $status = "1"
>
> Double quote around "$status" and drop double quote around "1"
> (which is clearly a literal string without any funnies) instead?
>
>> + then
>> + test_expect_success "git grep -f f $flags '$pattern_human' a" "
>> + printf '$pattern' | q_to_nul >f &&
>> + git grep -f f $flags a
>> + "
>> + elif test $status = "0"
>> + then
>> + test_expect_success "git grep -f f $flags '$pattern_human' a" "
>> + printf '$pattern' | q_to_nul >f &&
>> + test_must_fail git grep -f f $flags a
>> + "
All changed in v2.
> It somehow was unintuitive that 0 expected failure and 1 expected
> success, but it probably was just me.
Except this. The wildmatch uses the same idiom, and I think it makes
sense. 1 = true, 0 = false, not 0 = exit zero, 1 = exit nonzero, which
would also be IMO a bit more confusing since it should really be 0 and
!0 if you don't want to rely on specific non-zero exit codes, which is
just going down a garden path of complexity when all we wanted was
"does this match".