Re: [PATCH v10 2/3] t7507-commit-verbose: store output of grep in a file

>> > +! test -s out ||
>> > +rm out &&
>>
>> Why not just "rm -f out"? But, more importantly, why do you need to
>> remove the file at all? The '>' redirection operator (used below) will
>> overwrite the file, so no need to remove it beforehand.
>>
>> > +! grep '^diff --git' "$1" ||
>> > +grep '^diff --git' "$1" >out
>>
>> Um, what? Why two greps? I would have expected you to simply re-use
>> the existing grep (minus the backslash) while adding the redirection:
>>
>>     -exec grep '^diff --git' "\$1"
>>     +exec grep '^diff --git' "$1" >out
>>
>> Am I missing something obvious?
>
> In the non-verbose cases no diff is included in the commit message
> template, thus the pattern looking for it doesn't match anything, grep
> exits with error code, which in turn becomes the editor's exit
> code, ultimately making 'git commit' fail.  Not good.
>
> I suppose both the explicit 'rm out' and the '! grep ... || ...' is
> there to deal with this situation.

> Hmph.
>
> I think we could:
>
>   - either revive the idea of two editor scripts: one for the
>     non-verbose case checking with '! grep ...' that there are no
>     diffs in the commit message template, and one for all verbose
>     cases storing those diff lines in a file to be counted later.
>
>   - or use a fake editor that merely copies the whole commit message
>     template to a separate file, and we do the greping in the tests
>     themselves as well.
>
>   - or simply stick a 'true' at the end of the editor script ensuring
>     that it returns success even when grep can't find the pattern, but
>     I kind of feel ashamed of myself for even mentioning this
>     possibility ;)
>
> I would go for the second possibility, but don't feel strong about it.