David Aguilar schrieb am 27.10.2014 um 02:10: > On Sun, Oct 26, 2014 at 05:41:49PM -0700, David Aguilar wrote: >> On Sun, Oct 26, 2014 at 09:09:20AM +0100, Johannes Sixt wrote: >>> An attempt to quit difftool by hitting Ctrl-D (EOF) at its prompt does >>> not quit it, but is treated as if 'yes' was answered to the prompt and >>> all following prompts, which is contrary to the user's intent. Fix the >>> error check. >>> >>> Signed-off-by: Johannes Sixt <j6t@xxxxxxxx> >>> --- >>> Found while reviewing your latest patch. >> >> >> Thanks for the careful review. >> I have one small question about the test below. > > [snip] > >>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh >>> index dc30a51..9cf5dc9 100755 >>> --- a/t/t7800-difftool.sh >>> +++ b/t/t7800-difftool.sh >>> @@ -301,6 +301,14 @@ test_expect_success PERL 'say no to the second file' ' >>> ! grep br2 output >>> ' >>> >>> +test_expect_success PERL 'ending prompt input with EOF' ' >>> + git difftool -x cat branch </dev/null >output && >>> + ! grep master output && >>> + ! grep branch output && >>> + ! grep m2 output && >>> + ! grep br2 output >>> +' >> >> Should we use "test_must_fail grep ..." instead of "! grep ..." here? > > > Nevermind, this is good as-is. > Using "! grep" is consistent with the rest of the tests in t7800. > > What I'll do is add a follow-up patch in my upcoming reroll > that swaps all the "! grep" lines to "test_must_fail grep" > in one step. Don't do that ;) test_must_fail is meant for testing (git) commands such that a "failure return code" is marked as "success", whereas a failure to run the command is still capturd and marked as a failed test. For non-git commands like grep sed etc. which we do not perform tests *on* (but only *with*), the simple negator "!" is fine and preferable. Michael who has sinned in the past, but repented -- 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