> > +! 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.
--
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