{"thread":{"id":"37812","subject":"difftool--helper: exit when reading a prompt answer fails","startedAt":"2014-10-26T08:09:20Z","lastAt":"2014-10-28T09:34:09Z","messageCount":9,"participants":["Johannes Sixt","David Aguilar","Michael J Gruber","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"251056","messageId":"544CAC30.7000607@kdbg.org","threadId":"37812","inReplyTo":null,"subject":"difftool--helper: exit when reading a prompt answer fails","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-10-26T08:09:20Z","receivedAt":"2014-10-26T08:09:20Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"An attempt to quit difftool by hitting Ctrl-D (EOF) at its prompt does\nnot quit it, but is treated as if 'yes' was answered to the prompt and\nall following prompts, which is contrary to the user's intent. Fix the\nerror check.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n Found while reviewing your latest patch.\n\n I chose the 'foo || return' idiom for the error check, but left the\n 'if' for the interesting check, because I feel it is more readable\n than 'test ... && return'.\n\n -- Hannes\n\n git-difftool--helper.sh | 3 ++-\n t/t7800-difftool.sh     | 8 ++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 7ef36b9..aca0413 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -49,7 +49,8 @@ launch_merge_tool () {\n \t\telse\n \t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n \t\tfi\n-\t\tif read ans && test \"$ans\" = n\n+\t\tread ans || return\n+\t\tif test \"$ans\" = n\n \t\tthen\n \t\t\treturn\n \t\tfi\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex dc30a51..9cf5dc9 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -301,6 +301,14 @@ test_expect_success PERL 'say no to the second file' '\n \t! grep br2 output\n '\n \n+test_expect_success PERL 'ending prompt input with EOF' '\n+\tgit difftool -x cat branch </dev/null >output &&\n+\t! grep master output &&\n+\t! grep branch output &&\n+\t! grep m2 output &&\n+\t! grep br2 output\n+'\n+\n test_expect_success PERL 'difftool --tool-help' '\n \tgit difftool --tool-help >output &&\n \tgrep tool output\n-- \n2.0.0.12.gbcf935e\n"},{"id":"251080","messageId":"20141027004148.GB81127@gmail.com","threadId":"37812","inReplyTo":"544CAC30.7000607@kdbg.org","subject":"Re: difftool--helper: exit when reading a prompt answer fails","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-27T00:41:49Z","receivedAt":"2014-10-27T00:41:49Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, Oct 26, 2014 at 09:09:20AM +0100, Johannes Sixt wrote:\n> An attempt to quit difftool by hitting Ctrl-D (EOF) at its prompt does\n> not quit it, but is treated as if 'yes' was answered to the prompt and\n> all following prompts, which is contrary to the user's intent. Fix the\n> error check.\n> \n> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n> ---\n>  Found while reviewing your latest patch.\n\n\nThanks for the careful review.\nI have one small question about the test below.\n\nI'll base the reroll of my patch on top of this one so that\nI'm including your new test while developing.\n\nJunio, can you please queue this patch before my upcoming v3\npatch when I reroll?\n\nI also noticed that I forgot to \"git reset --hard\" after doing \"git add\"\nin one of the tests I introduced so I'll be adjusting that as well in v3.\n\n\n>  I chose the 'foo || return' idiom for the error check, but left the\n>  'if' for the interesting check, because I feel it is more readable\n>  than 'test ... && return'.\n> \n>  -- Hannes\n> \n>  git-difftool--helper.sh | 3 ++-\n>  t/t7800-difftool.sh     | 8 ++++++++\n>  2 files changed, 10 insertions(+), 1 deletion(-)\n> \n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 7ef36b9..aca0413 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -49,7 +49,8 @@ launch_merge_tool () {\n>  \t\telse\n>  \t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n>  \t\tfi\n> -\t\tif read ans && test \"$ans\" = n\n> +\t\tread ans || return\n> +\t\tif test \"$ans\" = n\n>  \t\tthen\n>  \t\t\treturn\n>  \t\tfi\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index dc30a51..9cf5dc9 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -301,6 +301,14 @@ test_expect_success PERL 'say no to the second file' '\n>  \t! grep br2 output\n>  '\n>  \n> +test_expect_success PERL 'ending prompt input with EOF' '\n> +\tgit difftool -x cat branch </dev/null >output &&\n> +\t! grep master output &&\n> +\t! grep branch output &&\n> +\t! grep m2 output &&\n> +\t! grep br2 output\n> +'\n\nShould we use \"test_must_fail grep ...\" instead of \"! grep ...\" here?\n-- \nDavid\n"},{"id":"251081","messageId":"20141027011051.GA87518@gmail.com","threadId":"37812","inReplyTo":"20141027004148.GB81127@gmail.com","subject":"Re: difftool--helper: exit when reading a prompt answer fails","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-27T01:10:52Z","receivedAt":"2014-10-27T01:10:52Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, Oct 26, 2014 at 05:41:49PM -0700, David Aguilar wrote:\n> On Sun, Oct 26, 2014 at 09:09:20AM +0100, Johannes Sixt wrote:\n> > An attempt to quit difftool by hitting Ctrl-D (EOF) at its prompt does\n> > not quit it, but is treated as if 'yes' was answered to the prompt and\n> > all following prompts, which is contrary to the user's intent. Fix the\n> > error check.\n> > \n> > Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n> > ---\n> >  Found while reviewing your latest patch.\n> \n> \n> Thanks for the careful review.\n> I have one small question about the test below.\n\n[snip]\n\n> > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> > index dc30a51..9cf5dc9 100755\n> > --- a/t/t7800-difftool.sh\n> > +++ b/t/t7800-difftool.sh\n> > @@ -301,6 +301,14 @@ test_expect_success PERL 'say no to the second file' '\n> >  \t! grep br2 output\n> >  '\n> >  \n> > +test_expect_success PERL 'ending prompt input with EOF' '\n> > +\tgit difftool -x cat branch </dev/null >output &&\n> > +\t! grep master output &&\n> > +\t! grep branch output &&\n> > +\t! grep m2 output &&\n> > +\t! grep br2 output\n> > +'\n> \n> Should we use \"test_must_fail grep ...\" instead of \"! grep ...\" here?\n\n\nNevermind, this is good as-is.\nUsing \"! grep\" is consistent with the rest of the tests in t7800.\n\nWhat I'll do is add a follow-up patch in my upcoming reroll\nthat swaps all the \"! grep\" lines to \"test_must_fail grep\"\nin one step.\n-- \nDavid\n"},{"id":"251082","messageId":"1414372542-3485-1-git-send-email-davvid@gmail.com","threadId":"37812","inReplyTo":"544CAC30.7000607@kdbg.org","subject":"[PATCH v3 1/2] t7800-difftool: use \"test_must_fail grep\" instead of \"! grep\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-27T01:15:41Z","receivedAt":"2014-10-27T01:15:41Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Signed-off-by: David Aguilar <davvid@gmail.com>\n---\nThis patch is new since v2.\n\nThis series now depends on \"difftool--helper: exit when reading a prompt answer fails\".\n\n t/t7800-difftool.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 9cf5dc9..b5b7f62 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -288,8 +288,8 @@ test_expect_success PERL 'say no to the first file' '\n \tgit difftool -x cat branch <input >output &&\n \tgrep m2 output &&\n \tgrep br2 output &&\n-\t! grep master output &&\n-\t! grep branch output\n+\ttest_must_fail grep master output &&\n+\ttest_must_fail grep branch output\n '\n \n test_expect_success PERL 'say no to the second file' '\n@@ -297,16 +297,16 @@ test_expect_success PERL 'say no to the second file' '\n \tgit difftool -x cat branch <input >output &&\n \tgrep master output &&\n \tgrep branch output &&\n-\t! grep m2 output &&\n-\t! grep br2 output\n+\ttest_must_fail grep m2 output &&\n+\ttest_must_fail grep br2 output\n '\n \n test_expect_success PERL 'ending prompt input with EOF' '\n \tgit difftool -x cat branch </dev/null >output &&\n-\t! grep master output &&\n-\t! grep branch output &&\n-\t! grep m2 output &&\n-\t! grep br2 output\n+\ttest_must_fail grep master output &&\n+\ttest_must_fail grep branch output &&\n+\ttest_must_fail grep m2 output &&\n+\ttest_must_fail grep br2 output\n '\n \n test_expect_success PERL 'difftool --tool-help' '\n-- \n2.1.2.557.g1025af0\n"},{"id":"251083","messageId":"1414372542-3485-2-git-send-email-davvid@gmail.com","threadId":"37812","inReplyTo":"1414372542-3485-1-git-send-email-davvid@gmail.com","subject":"[PATCH v3 2/2] difftool: add support for --trust-exit-code","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-27T01:15:42Z","receivedAt":"2014-10-27T01:15:42Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Teach difftool to exit when a diff tool returns a non-zero exit\ncode when either --trust-exit-code is specified or\ndifftool.trustExitCode is true.\n\nForward exit codes from invoked diff tools to the caller when\n--trust-exit-code is used.\n\nSuggested-by: Adri Farr <14farresa@gmail.com>\nHelped-by: Johannes Sixt <j6t@kdbg.org>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nChanges since v2:\n\nThis series now depends on \"difftool--helper: exit when reading a prompt answer fails\".\n\nA missing \"git reset -- for-diff\" was missing from the test.\n\nSetting and testing of the status variable was moved to be\nbefore \"shift 7\" to make things more cohesive; status setting\nwas previously buried inside the launch_merge_tool function,\nwhich made it harder to follow.\n\n Documentation/git-difftool.txt | 14 ++++++++++++++\n git-difftool--helper.sh        |  6 ++++++\n git-difftool.perl              | 12 ++++++++++++\n t/t7800-difftool.sh            | 44 ++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 76 insertions(+)\n\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex 11887e6..333cf6f 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -91,6 +91,15 @@ instead.  `--no-symlinks` is the default on Windows.\n \tthe default diff tool will be read from the configured\n \t`diff.guitool` variable instead of `diff.tool`.\n \n+--[no-]trust-exit-code::\n+\t'git-difftool' invokes a diff tool individually on each file.\n+\tErrors reported by the diff tool are ignored by default.\n+\tUse `--trust-exit-code` to make 'git-difftool' exit when an\n+\tinvoked diff tool returns a non-zero exit code.\n++\n+'git-difftool' will forward the exit code of the invoked tool when\n+'--trust-exit-code' is used.\n+\n See linkgit:git-diff[1] for the full list of supported options.\n \n CONFIG VARIABLES\n@@ -116,6 +125,11 @@ See the `--tool=<tool>` option above for more details.\n difftool.prompt::\n \tPrompt before each invocation of the diff tool.\n \n+difftool.trustExitCode::\n+\tExit difftool if the invoked diff tool returns a non-zero exit status.\n++\n+See the `--trust-exit-code` option above for more details.\n+\n SEE ALSO\n --------\n linkgit:git-diff[1]::\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex aca0413..d4fb6df 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -85,6 +85,12 @@ else\n \twhile test $# -gt 6\n \tdo\n \t\tlaunch_merge_tool \"$1\" \"$2\" \"$5\"\n+\t\tstatus=$?\n+\t\tif test \"$status\" != 0 &&\n+\t\t\ttest \"$GIT_DIFFTOOL_TRUST_EXIT_CODE\" = true\n+\t\tthen\n+\t\t\texit $status\n+\t\tfi\n \t\tshift 7\n \tdone\n fi\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex 598fcc2..7df7c8a 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -342,6 +342,7 @@ sub main\n \t\tsymlinks => $^O ne 'cygwin' &&\n \t\t\t\t$^O ne 'MSWin32' && $^O ne 'msys',\n \t\ttool_help => undef,\n+\t\ttrust_exit_code => undef,\n \t);\n \tGetOptions('g|gui!' => \\$opts{gui},\n \t\t'd|dir-diff' => \\$opts{dirdiff},\n@@ -352,6 +353,8 @@ sub main\n \t\t'no-symlinks' => sub { $opts{symlinks} = 0; },\n \t\t't|tool:s' => \\$opts{difftool_cmd},\n \t\t'tool-help' => \\$opts{tool_help},\n+\t\t'trust-exit-code' => \\$opts{trust_exit_code},\n+\t\t'no-trust-exit-code' => sub { $opts{trust_exit_code} = 0; },\n \t\t'x|extcmd:s' => \\$opts{extcmd});\n \n \tif (defined($opts{help})) {\n@@ -383,6 +386,15 @@ sub main\n \t\t}\n \t}\n \n+\tif (!defined $opts{trust_exit_code}) {\n+\t\t$opts{trust_exit_code} = Git::config_bool('difftool.trustExitCode');\n+\t}\n+\tif ($opts{trust_exit_code}) {\n+\t\t$ENV{GIT_DIFFTOOL_TRUST_EXIT_CODE} = 'true';\n+\t} else {\n+\t\t$ENV{GIT_DIFFTOOL_TRUST_EXIT_CODE} = 'false';\n+\t}\n+\n \t# In directory diff mode, 'git-difftool--helper' is called once\n \t# to compare the a/b directories.  In file diff mode, 'git diff'\n \t# will invoke a separate instance of 'git-difftool--helper' for\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex b5b7f62..7de18dd 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -76,6 +76,50 @@ test_expect_success PERL 'difftool forwards arguments to diff' '\n \trm for-diff\n '\n \n+test_expect_success PERL 'difftool ignores exit code' '\n+\ttest_config difftool.error.cmd false &&\n+\tgit difftool -y -t error branch\n+'\n+\n+test_expect_success PERL 'difftool forwards exit code with --trust-exit-code' '\n+\ttest_config difftool.error.cmd false &&\n+\ttest_must_fail git difftool -y --trust-exit-code -t error branch\n+'\n+\n+test_expect_success PERL 'difftool honors difftool.trustExitCode = true' '\n+\ttest_config difftool.error.cmd false &&\n+\ttest_config difftool.trustExitCode true &&\n+\ttest_must_fail git difftool -y -t error branch\n+'\n+\n+test_expect_success PERL 'difftool honors difftool.trustExitCode = false' '\n+\ttest_config difftool.error.cmd false &&\n+\ttest_config difftool.trustExitCode false &&\n+\tgit difftool -y -t error branch\n+'\n+\n+test_expect_success PERL 'difftool ignores exit code with --no-trust-exit-code' '\n+\ttest_config difftool.error.cmd false &&\n+\ttest_config difftool.trustExitCode true &&\n+\tgit difftool -y --no-trust-exit-code -t error branch\n+'\n+\n+write_script .git/fail-right-file <<\\EOF\n+echo \"$2\"\n+exit 1\n+EOF\n+\n+test_expect_success PERL 'difftool stops on error with --trust-exit-code' '\n+\t>for-diff &&\n+\tgit add for-diff &&\n+\techo file>expect &&\n+\ttest_must_fail git difftool -y --trust-exit-code \\\n+\t\t--extcmd .git/fail-right-file branch >actual &&\n+\ttest_cmp expect actual &&\n+\tgit reset -- for-diff &&\n+\trm -f for-diff .git/fail-right-file\n+'\n+\n test_expect_success PERL 'difftool honors --gui' '\n \tdifftool_test_setup &&\n \ttest_config merge.tool bogus-tool &&\n-- \n2.1.2.557.g1025af0\n"},{"id":"251116","messageId":"544E4077.8070208@drmicha.warpmail.net","threadId":"37812","inReplyTo":"20141027011051.GA87518@gmail.com","subject":"Re: difftool--helper: exit when reading a prompt answer fails","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-10-27T12:54:15Z","receivedAt":"2014-10-27T12:54:15Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"David Aguilar schrieb am 27.10.2014 um 02:10:\n> On Sun, Oct 26, 2014 at 05:41:49PM -0700, David Aguilar wrote:\n>> On Sun, Oct 26, 2014 at 09:09:20AM +0100, Johannes Sixt wrote:\n>>> An attempt to quit difftool by hitting Ctrl-D (EOF) at its prompt does\n>>> not quit it, but is treated as if 'yes' was answered to the prompt and\n>>> all following prompts, which is contrary to the user's intent. Fix the\n>>> error check.\n>>>\n>>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n>>> ---\n>>>  Found while reviewing your latest patch.\n>>\n>>\n>> Thanks for the careful review.\n>> I have one small question about the test below.\n> \n> [snip]\n> \n>>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n>>> index dc30a51..9cf5dc9 100755\n>>> --- a/t/t7800-difftool.sh\n>>> +++ b/t/t7800-difftool.sh\n>>> @@ -301,6 +301,14 @@ test_expect_success PERL 'say no to the second file' '\n>>>  \t! grep br2 output\n>>>  '\n>>>  \n>>> +test_expect_success PERL 'ending prompt input with EOF' '\n>>> +\tgit difftool -x cat branch </dev/null >output &&\n>>> +\t! grep master output &&\n>>> +\t! grep branch output &&\n>>> +\t! grep m2 output &&\n>>> +\t! grep br2 output\n>>> +'\n>>\n>> Should we use \"test_must_fail grep ...\" instead of \"! grep ...\" here?\n> \n> \n> Nevermind, this is good as-is.\n> Using \"! grep\" is consistent with the rest of the tests in t7800.\n> \n> What I'll do is add a follow-up patch in my upcoming reroll\n> that swaps all the \"! grep\" lines to \"test_must_fail grep\"\n> in one step.\n\nDon't do that ;)\n\ntest_must_fail is meant for testing (git) commands such that a \"failure\nreturn code\" is marked as \"success\", whereas a failure to run the\ncommand is still capturd and marked as a failed test.\n\nFor non-git commands like grep sed etc. which we do not perform tests\n*on* (but only *with*), the simple negator \"!\" is fine and preferable.\n\nMichael who has sinned in the past, but repented\n"},{"id":"251122","messageId":"xmqqa94hfm60.fsf@gitster.dls.corp.google.com","threadId":"37812","inReplyTo":"20141027004148.GB81127@gmail.com","subject":"Re: difftool--helper: exit when reading a prompt answer fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-27T17:38:15Z","receivedAt":"2014-10-27T17:38:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n>> index dc30a51..9cf5dc9 100755\n>> --- a/t/t7800-difftool.sh\n>> +++ b/t/t7800-difftool.sh\n>> @@ -301,6 +301,14 @@ test_expect_success PERL 'say no to the second file' '\n>>  \t! grep br2 output\n>>  '\n>>  \n>> +test_expect_success PERL 'ending prompt input with EOF' '\n>> +\tgit difftool -x cat branch </dev/null >output &&\n>> +\t! grep master output &&\n>> +\t! grep branch output &&\n>> +\t! grep m2 output &&\n>> +\t! grep br2 output\n>> +'\n>\n> Should we use \"test_must_fail grep ...\" instead of \"! grep ...\" here?\n\nNO.  We do not expect system-supplied \"grep\" to dump core and\ndeclare it as a test failure.  test_must_fail is for catching an\nexpected non-zero status exit from git commands, i.e. when we expect\nour binary to correctly notice some condition and report that fact\nwith non-zero exit status, we do not want to mistake the binary\nsegfaulting as working correctly.\n"},{"id":"251125","messageId":"xmqq38a9tkqt.fsf@gitster.dls.corp.google.com","threadId":"37812","inReplyTo":"1414372542-3485-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v3 2/2] difftool: add support for --trust-exit-code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-27T18:45:14Z","receivedAt":"2014-10-27T18:45:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> +write_script .git/fail-right-file <<\\EOF\n> +echo \"$2\"\n> +exit 1\n> +EOF\n\nThis should be inside the next one, no?\n\n> +test_expect_success PERL 'difftool stops on error with --trust-exit-code' '\n> +\t>for-diff &&\n> +\tgit add for-diff &&\n> +\techo file>expect &&\n> +\ttest_must_fail git difftool -y --trust-exit-code \\\n> +\t\t--extcmd .git/fail-right-file branch >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit reset -- for-diff &&\n> +\trm -f for-diff .git/fail-right-file\n> +'\n\nIn other words, this squashed in.\n\n t/t7800-difftool.sh | 17 ++++++++---------\n 1 file changed, 8 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 4b2f611..69bde7a 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -104,20 +104,19 @@ test_expect_success PERL 'difftool ignores exit code with --no-trust-exit-code'\n \tgit difftool -y --no-trust-exit-code -t error branch\n '\n \n-write_script .git/fail-right-file <<\\EOF\n-echo \"$2\"\n-exit 1\n-EOF\n-\n test_expect_success PERL 'difftool stops on error with --trust-exit-code' '\n+\ttest_when_finished \"rm -f for-diff .git/fail-right-file\" &&\n+\ttest_when_finished \"git reset -- for-diff\" &&\n+\twrite_script .git/fail-right-file <<-\\EOF &&\n+\techo \"$2\"\n+\texit 1\n+\tEOF\n \t>for-diff &&\n \tgit add for-diff &&\n-\techo file>expect &&\n+\techo file >expect &&\n \ttest_must_fail git difftool -y --trust-exit-code \\\n \t\t--extcmd .git/fail-right-file branch >actual &&\n-\ttest_cmp expect actual &&\n-\tgit reset -- for-diff &&\n-\trm -f for-diff .git/fail-right-file\n+\ttest_cmp expect actual\n '\n \n test_expect_success PERL 'difftool honors --gui' '\n"},{"id":"251144","messageId":"20141028093407.GA20944@gmail.com","threadId":"37812","inReplyTo":"xmqq38a9tkqt.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/2] difftool: add support for --trust-exit-code","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-28T09:34:09Z","receivedAt":"2014-10-28T09:34:09Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Mon, Oct 27, 2014 at 11:45:14AM -0700, Junio C Hamano wrote:\n> David Aguilar <davvid@gmail.com> writes:\n> \n> > +write_script .git/fail-right-file <<\\EOF\n> > +echo \"$2\"\n> > +exit 1\n> > +EOF\n> \n> This should be inside the next one, no?\n> \n> > +test_expect_success PERL 'difftool stops on error with --trust-exit-code' '\n> > +\t>for-diff &&\n> > +\tgit add for-diff &&\n> > +\techo file>expect &&\n> > +\ttest_must_fail git difftool -y --trust-exit-code \\\n> > +\t\t--extcmd .git/fail-right-file branch >actual &&\n> > +\ttest_cmp expect actual &&\n> > +\tgit reset -- for-diff &&\n> > +\trm -f for-diff .git/fail-right-file\n> > +'\n> \n> In other words, this squashed in.\n\n\nYes, please.  And drop patch 1/2, of course.\nThanks for the clarification.\n\n\n> \n>  t/t7800-difftool.sh | 17 ++++++++---------\n>  1 file changed, 8 insertions(+), 9 deletions(-)\n> \n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index 4b2f611..69bde7a 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -104,20 +104,19 @@ test_expect_success PERL 'difftool ignores exit code with --no-trust-exit-code'\n>  \tgit difftool -y --no-trust-exit-code -t error branch\n>  '\n>  \n> -write_script .git/fail-right-file <<\\EOF\n> -echo \"$2\"\n> -exit 1\n> -EOF\n> -\n>  test_expect_success PERL 'difftool stops on error with --trust-exit-code' '\n> +\ttest_when_finished \"rm -f for-diff .git/fail-right-file\" &&\n> +\ttest_when_finished \"git reset -- for-diff\" &&\n> +\twrite_script .git/fail-right-file <<-\\EOF &&\n> +\techo \"$2\"\n> +\texit 1\n> +\tEOF\n>  \t>for-diff &&\n>  \tgit add for-diff &&\n> -\techo file>expect &&\n> +\techo file >expect &&\n>  \ttest_must_fail git difftool -y --trust-exit-code \\\n>  \t\t--extcmd .git/fail-right-file branch >actual &&\n> -\ttest_cmp expect actual &&\n> -\tgit reset -- for-diff &&\n> -\trm -f for-diff .git/fail-right-file\n> +\ttest_cmp expect actual\n>  '\n>  \n>  test_expect_success PERL 'difftool honors --gui' '\n\n-- \nDavid\n"}]}