Merge branch 'jk/diff-convfilter'

* jk/diff-convfilter:
enable textconv for diff in verbose status/commit
wt-status: load diff ui config
only textconv regular files
userdiff: require explicitly allowing textconv
refactor userdiff textconv code

Conflicts:
t/t4030-diff-textconv.sh

diff --git a/builtin-diff.c b/builtin-diff.c
index 82d4ddabd85072829199fe8dc17c2de15691dac2..7ceceeb6ff49d60468293941b5f37141e8fc01b9 100644 (file)
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -300,6 +300,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
        }
        DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
        DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+       DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
 
        /*
         * If the user asked for our exit code then don't start a

diff --git a/builtin-log.c b/builtin-log.c
index a0944f70a4a3aa4f5fb08c4ac4ae7dc31d25d532..75d698f0cef491b2c7fdc4920b37cb8e61da5e20 100644 (file)
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -59,6 +59,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
                } else
                        die("unrecognized argument: %s", arg);
        }
+       DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);
 }
 
 /*

diff --git a/diff.c b/diff.c
index 3e6c752bb77109864f8be170f9b0e2504531b2aa..f644947c823e824a294414a660beac02b4182fe4 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -93,12 +93,6 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
        if (!strcmp(var, "diff.external"))
                return git_config_string(&external_diff_cmd_cfg, var, value);
 
-       switch (userdiff_config_porcelain(var, value)) {
-               case 0: break;
-               case -1: return -1;
-               default: return 0;
-       }
-
        return git_diff_basic_config(var, value, cb);
 }
 
@@ -109,6 +103,12 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
+       switch (userdiff_config(var, value)) {
+               case 0: break;
+               case -1: return -1;
+               default: return 0;
+       }
+
        if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
                int slot = parse_diff_color_slot(var, 11);
                if (!value)
@@ -123,12 +123,6 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
-       switch (userdiff_config_basic(var, value)) {
-               case 0: break;
-               case -1: return -1;
-               default: return 0;
-       }
-
        return git_color_default_config(var, value, cb);
 }
 
@@ -294,18 +288,8 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
        else if (diff_populate_filespec(one, 0))
                return -1;
 
-       diff_filespec_load_driver(one);
-       if (one->driver->textconv) {
-               size_t size;
-               mf->ptr = run_textconv(one->driver->textconv, one, &size);
-               if (!mf->ptr)
-                       return -1;
-               mf->size = size;
-       }
-       else {
-               mf->ptr = one->data;
-               mf->size = one->size;
-       }
+       mf->ptr = one->data;
+       mf->size = one->size;
        return 0;
 }
 
@@ -1324,6 +1308,16 @@ void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const
                options->b_prefix = b;
 }
 
+static const char *get_textconv(struct diff_filespec *one)
+{
+       if (!DIFF_FILE_VALID(one))
+               return NULL;
+       if (!S_ISREG(one->mode))
+               return NULL;
+       diff_filespec_load_driver(one);
+       return one->driver->textconv;
+}
+
 static void builtin_diff(const char *name_a,
                         const char *name_b,
                         struct diff_filespec *one,
@@ -1338,6 +1332,7 @@ static void builtin_diff(const char *name_a,
        const char *set = diff_get_color_opt(o, DIFF_METAINFO);
        const char *reset = diff_get_color_opt(o, DIFF_RESET);
        const char *a_prefix, *b_prefix;
+       const char *textconv_one = NULL, *textconv_two = NULL;
 
        diff_set_mnemonic_prefix(o, "a/", "b/");
        if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
@@ -1391,8 +1386,14 @@ static void builtin_diff(const char *name_a,
        if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                die("unable to read files to diff");
 
+       if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+               textconv_one = get_textconv(one);
+               textconv_two = get_textconv(two);
+       }
+
        if (!DIFF_OPT_TST(o, TEXT) &&
-           (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {
+           ( (diff_filespec_is_binary(one) && !textconv_one) ||
+             (diff_filespec_is_binary(two) && !textconv_two) )) {
                /* Quite common confusing case */
                if (mf1.size == mf2.size &&
                    !memcmp(mf1.ptr, mf2.ptr, mf1.size))
@@ -1413,6 +1414,21 @@ static void builtin_diff(const char *name_a,
                struct emit_callback ecbdata;
                const struct userdiff_funcname *pe;
 
+               if (textconv_one) {
+                       size_t size;
+                       mf1.ptr = run_textconv(textconv_one, one, &size);
+                       if (!mf1.ptr)
+                               die("unable to read files to diff");
+                       mf1.size = size;
+               }
+               if (textconv_two) {
+                       size_t size;
+                       mf2.ptr = run_textconv(textconv_two, two, &size);
+                       if (!mf2.ptr)
+                               die("unable to read files to diff");
+                       mf2.size = size;
+               }
+
                pe = diff_funcname_pattern(one);
                if (!pe)
                        pe = diff_funcname_pattern(two);
@@ -1445,6 +1461,10 @@ static void builtin_diff(const char *name_a,
                              &xpp, &xecfg, &ecb);
                if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
                        free_diff_words_data(&ecbdata);
+               if (textconv_one)
+                       free(mf1.ptr);
+               if (textconv_two)
+                       free(mf2.ptr);
        }
 
  free_ab_and_return:

diff --git a/diff.h b/diff.h
index a49d865bd9cb0fa5ff27ccad7049074afb0002e9..42582edee68a4a4717ae5debebf37e6b9610fc8f 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -65,6 +65,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_IGNORE_SUBMODULES   (1 << 18)
 #define DIFF_OPT_DIRSTAT_CUMULATIVE  (1 << 19)
 #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)
+#define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
 #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)

diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 3aed1bbdfed0f415a615f77c72210336d83f33ed..03ba26a0de7d1bf41269cfb1f1b9f33b733482e1 100755 (executable)
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -52,7 +52,7 @@ test_expect_success 'setup textconv filters' '
        git config diff.fail.textconv false
 '
 
-test_expect_failure 'diff produces text' '
+test_expect_success 'diff produces text' '
        git diff HEAD^ HEAD >diff &&
        find_diff <diff >actual &&
        test_cmp expect.text actual
@@ -64,23 +64,31 @@ test_expect_success 'diff-tree produces binary' '
        test_cmp expect.binary actual
 '
 
-test_expect_failure 'log produces text' '
+test_expect_success 'log produces text' '
        git log -1 -p >log &&
        find_diff <log >actual &&
        test_cmp expect.text actual
 '
 
-test_expect_failure 'format-patch produces binary' '
+test_expect_success 'format-patch produces binary' '
        git format-patch --no-binary --stdout HEAD^ >patch &&
        find_diff <patch >actual &&
        test_cmp expect.binary actual
 '
 
+test_expect_success 'status -v produces text' '
+       git reset --soft HEAD^ &&
+       git status -v >diff &&
+       find_diff <diff >actual &&
+       test_cmp expect.text actual &&
+       git reset --soft HEAD@{1}
+'
+
 cat >expect.stat <<'EOF'
  file |  Bin 2 -> 4 bytes
  1 files changed, 0 insertions(+), 0 deletions(-)
 EOF
-test_expect_failure 'diffstat does not run textconv' '
+test_expect_success 'diffstat does not run textconv' '
        echo file diff=fail >.gitattributes &&
        git diff --stat HEAD^ HEAD >actual &&
        test_cmp expect.stat actual
@@ -104,7 +112,7 @@ index ad8b3d2..67be421
 \ No newline at end of file
 EOF
 # make a symlink the hard way that works on symlink-challenged file systems
-test_expect_failure 'textconv does not act on symlinks' '
+test_expect_success 'textconv does not act on symlinks' '
        printf frotz > file &&
        git add file &&
        git ls-files -s | sed -e s/100644/120000/ |

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 3eb9faedcf75c7b8a535b369e5a19107c6e81026..ad42c78d7c21497a6ebb4e9cef4efd6334f947da 100755 (executable)
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -89,6 +89,14 @@ test_expect_success 'verbose' '
 
 '
 
+test_expect_success 'verbose respects diff config' '
+
+       git config color.diff always &&
+       git status -v >actual &&
+       grep "\[1mdiff --git" actual &&
+       git config --unset color.diff
+'
+
 test_expect_success 'cleanup commit messages (verbatim,-t)' '
 
        echo >>negative &&

diff --git a/userdiff.c b/userdiff.c
index d95257ab3bd4fafa59b3ed2aa58c480de274227d..3681062ebfef85af08d71ed6e1ff734804906d6a 100644 (file)
--- a/userdiff.c
+++ b/userdiff.c
@@ -120,7 +120,7 @@ static int parse_tristate(int *b, const char *k, const char *v)
        return 1;
 }
 
-int userdiff_config_basic(const char *k, const char *v)
+int userdiff_config(const char *k, const char *v)
 {
        struct userdiff_driver *drv;
 
@@ -130,14 +130,6 @@ int userdiff_config_basic(const char *k, const char *v)
                return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
        if ((drv = parse_driver(k, v, "binary")))
                return parse_tristate(&drv->binary, k, v);
-
-       return 0;
-}
-
-int userdiff_config_porcelain(const char *k, const char *v)
-{
-       struct userdiff_driver *drv;
-
        if ((drv = parse_driver(k, v, "command")))
                return parse_string(&drv->external, k, v);
        if ((drv = parse_driver(k, v, "textconv")))

diff --git a/userdiff.h b/userdiff.h
index f29c18ffb302dc009d438a08800c4aa202fd2f12..ba2945770b379f51aa8da45d112a2ef896ec4c10 100644 (file)
--- a/userdiff.h
+++ b/userdiff.h
@@ -14,8 +14,7 @@ struct userdiff_driver {
        const char *textconv;
 };
 
-int userdiff_config_basic(const char *k, const char *v);
-int userdiff_config_porcelain(const char *k, const char *v);
+int userdiff_config(const char *k, const char *v);
 struct userdiff_driver *userdiff_find_by_name(const char *name);
 struct userdiff_driver *userdiff_find_by_path(const char *path);
 

diff --git a/wt-status.c b/wt-status.c
index c3a9cab8980daa38e647a13eb2f0fcb42fbfbb8e..6a7645ed86fd5879e959460011a8add015d392d9 100644 (file)
--- a/wt-status.c
+++ b/wt-status.c
@@ -301,8 +301,17 @@ static void wt_status_print_verbose(struct wt_status *s)
        setup_revisions(0, NULL, &rev, s->reference);
        rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
        rev.diffopt.detect_rename = 1;
+       DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
        rev.diffopt.file = s->fp;
        rev.diffopt.close_file = 0;
+       /*
+        * If we're not going to stdout, then we definitely don't
+        * want color, since we are going to the commit message
+        * file (and even the "auto" setting won't work, since it
+        * will have checked isatty on stdout).
+        */
+       if (s->fp != stdout)
+               DIFF_OPT_CLR(&rev.diffopt, COLOR_DIFF);
        run_diff_index(&rev, 1);
 }
 
@@ -422,5 +431,5 @@ int git_status_config(const char *k, const char *v, void *cb)
                        return error("Invalid untracked files mode '%s'", v);
                return 0;
        }
-       return git_color_default_config(k, v, cb);
+       return git_diff_ui_config(k, v, cb);
 }