diff: correctly disable external_diff with --no-ext-diff

Upon seeing a type-change filepair, "diff --no-ext-diff" does not
show the usual "deletion followed by addition" split patch and does
not run the external diff driver either.

This is because the logic to disable external diff was placed at a
wrong level in the callchain.  run_diff_cmd() decides to show the
split patch only when external diff driver is not configured or
specified via GIT_EXTERNAL_DIFF environment, but this is done before
checking if --no-ext-diff was given.  To make things worse,
run_diff_cmd() checks --no-ext-diff and disables the output for such
a filepair completely, as the callchain below it (e.g. builtin_diff)
does not want to handle typechange filepairs.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 5388ded2140d9d4268fbda702603902bf4530981..f4b58709ebc872e31b028abafe7ca2b79aa70aab 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -2913,9 +2913,8 @@ static void run_diff_cmd(const char *pgm,
        int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
        int must_show_header = 0;
 
        int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
        int must_show_header = 0;
 

-       if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
-               pgm = NULL;
-       else {
+
+       if (DIFF_OPT_TST(o, ALLOW_EXTERNAL)) {

                struct userdiff_driver *drv = userdiff_find_by_path(attr_path);
                if (drv && drv->external)
                        pgm = drv->external;
                struct userdiff_driver *drv = userdiff_find_by_path(attr_path);
                if (drv && drv->external)
                        pgm = drv->external;


@@ -2995,6 +2994,9 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
        if (o->prefix_length)
                strip_prefix(o->prefix_length, &name, &other);
 
        if (o->prefix_length)
                strip_prefix(o->prefix_length, &name, &other);
 

+       if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
+               pgm = NULL;
+

        if (DIFF_PAIR_UNMERGED(p)) {
                run_diff_cmd(pgm, name, NULL, attr_path,
                             NULL, NULL, NULL, o, p);
        if (DIFF_PAIR_UNMERGED(p)) {
                run_diff_cmd(pgm, name, NULL, attr_path,
                             NULL, NULL, NULL, o, p);

diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index 083f62d1d6bb6bfd908b0a03db1a47f80071ef91..025df533ca8969e67c38f8dcb913c9021b816c57 100755 (executable)
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -48,7 +48,26 @@ test_expect_success 'GIT_EXTERNAL_DIFF environment and --no-ext-diff' '
 
 '
 
 
 '
 

+test_expect_success SYMLINKS 'typechange diff' '
+       rm -f file &&
+       ln -s elif file &&
+       GIT_EXTERNAL_DIFF=echo git diff  | {
+               read path oldfile oldhex oldmode newfile newhex newmode &&
+               test "z$path" = zfile &&
+               test "z$oldmode" = z100644 &&
+               test "z$newhex" = "z$_z40" &&
+               test "z$newmode" = z120000 &&
+               oh=$(git rev-parse --verify HEAD:file) &&
+               test "z$oh" = "z$oldhex"
+       } &&
+       GIT_EXTERNAL_DIFF=echo git diff --no-ext-diff >actual &&
+       git diff >expect &&
+       test_cmp expect actual
+'
+

 test_expect_success 'diff attribute' '
 test_expect_success 'diff attribute' '

+       git reset --hard &&
+       echo third >file &&

 
        git config diff.parrot.command echo &&
 
 
        git config diff.parrot.command echo &&