From: Torsten Bögershausen <tboegi@web.de>
Date: Sat, 22 Nov 2014 14:59:12 GMT
Subject: Re: [PATCHv2] add: ignore only ignored files
Message-ID: <5470A4C0.3070501@web.de>
In-Reply-To: <20141121180105.GB26650@peff.net>

>> +test_expect_success 'error out when attempting to add ignored ones but add others' '
>> +	touch a.if &&
>> +	test_must_fail git add a.?? &&
>> +	! (git ls-files | grep "\\.ig") &&
>> +	(git ls-files | grep a.if)
>> +'
> 
> I am somewhat allergic to pipes in our test suite, because they can mask
> errors (especially with a negated grep, because we do not know if they
> correctly produced any output at all). But I guess this is matching the
> surrounding code, and it is quite unlikely for `ls-files` to fail in any
> meaningful way here. So I think it's fine.
> 
> -Peff

2 small comments:
Why the escaped "\\.ig" and the unescaped "a.if"  ?

The other question, this is a more general one, strikes me every time I see
! grep

Should we avoid it by writing "test_must_fail" instead of "!" ?
(The current code base has a mixture of both)

The following came into my mind when working on another grepy thing,
and it may be unnecessary clumsy:

test_expect_success 'error out when attempting to add ignored ones but add others' '
	touch a.if &&
	test_must_fail git add a.?? &&
	git ls-files >files.txt &&
	test_must_fail grep a.ig files.txt >/dev/null &&
	grep a.if files.txt >/dev/null &&
	rm files.txt
'

(It feels as if there should be a "grepnot" ;-)


The 3rd comment:
Thanks for taking this up!