run_external_diff: clean up error handling
authorJeff King <peff@peff.net>
Sat, 19 Apr 2014 19:19:19 +0000 (15:19 -0400)
committerJunio C Hamano <gitster@pobox.com>
Mon, 21 Apr 2014 17:31:36 +0000 (10:31 -0700)
When the external diff reports an error, we try to clean up
and die. However, we can make this process a bit simpler:

1. We do not need to bother freeing memory, since we are
about to exit. Nor do we need to clean up our
tempfiles, since the atexit() handler will do it for
us. So we can die as soon as we see the error.

3. We can just call die() rather than fprintf/exit. This
does technically change our exit code, but the exit
code of "1" is not meaningful here. In fact, it is
probably wrong, since "1" from diff usually means
"completed successfully, but there were differences".

And while we're there, we can mark the error message for
translation, and drop the full stop at the end to make it
more like our other messages.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff.c
t/t7800-difftool.sh
diff --git a/diff.c b/diff.c
index c7b433ffcd8a8df847ebfcd4b4a3e504059a4d73..174f146d0d89e03d4f603c0c2d7cd9153de14388 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -2909,7 +2909,6 @@ static void run_external_diff(const char *pgm,
 {
        struct argv_array argv = ARGV_ARRAY_INIT;
        struct argv_array env = ARGV_ARRAY_INIT;
-       int retval;
        struct diff_queue_struct *q = &diff_queued_diff;
 
        if (one && two) {
@@ -2938,14 +2937,12 @@ static void run_external_diff(const char *pgm,
        argv_array_pushf(&env, "GIT_DIFF_PATH_COUNTER=%d", ++o->diff_path_counter);
        argv_array_pushf(&env, "GIT_DIFF_PATH_TOTAL=%d", q->nr);
 
-       retval = run_command_v_opt_cd_env(argv.argv, RUN_USING_SHELL, NULL, env.argv);
+       if (run_command_v_opt_cd_env(argv.argv, RUN_USING_SHELL, NULL, env.argv))
+               die(_("external diff died, stopping at %s"), name);
+
        remove_tempfile();
        argv_array_clear(&argv);
        argv_array_clear(&env);
-       if (retval) {
-               fprintf(stderr, "external diff died, stopping at %s.\n", name);
-               exit(1);
-       }
 }
 
 static int similarity_index(struct diff_filepair *p)
index 5a193c500d282cc2b13e8de6e128229585897a16..dc30a514bf681dde44dea32e671632ef6f0a47a7 100755 (executable)
@@ -58,7 +58,7 @@ test_expect_success PERL 'custom tool commands override built-ins' '
 
 test_expect_success PERL 'difftool ignores bad --tool values' '
        : >expect &&
-       test_expect_code 1 \
+       test_must_fail \
                git difftool --no-prompt --tool=bad-tool branch >actual &&
        test_cmp expect actual
 '