On Sun, Mar 27, 2016 at 6:57 PM, SZEDER Gábor <szeder@xxxxxxxxxx> wrote: >> > +! 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. Yes. In fact, I did this as a last resort after trying a lot of other stuff which didn't work. > 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. There is one more possibility, that we could use 'test_must_fail' (as grep exits with error code with no diff) but this will send a very wrong interpretation as "This type of scenario is not meant to work". Or to put it in better words, "You cannot have no diff with commit". There is a difference between two negatives and one positive. -- 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