pickaxe: use textconv for -S counting

We currently just look at raw blob data when using "-S" to
pickaxe. This is mostly historical, as pickaxe predates the
textconv feature. If the user has bothered to define a
textconv filter, it is more likely that their search string will be
on the textconv output, as that is what they will see in the
diff (and we do not even provide a mechanism for them to
search for binary needles that contain NUL characters).

This patch teaches "-S" to use textconv, just as we
already do for "-G".

Signed-off-by: Jeff King <peff@peff.net>

diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 61f628c28579a0350065ae0b7725624625e03ba5..b097fa766177f90e4b7983789a9286506664b2fd 100644 (file)
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -157,17 +157,15 @@ static void diffcore_pickaxe_grep(struct diff_options *o)
        return;
 }
 
-static unsigned int contains(struct diff_filespec *one, struct diff_options *o,
+static unsigned int contains(mmfile_t *mf, struct diff_options *o,
                             regex_t *regexp, kwset_t kws)
 {
        unsigned int cnt;
        unsigned long sz;
        const char *data;
-       if (diff_populate_filespec(one, 0))
-               return 0;
 
-       sz = one->size;
-       data = one->data;
+       sz = mf->size;
+       data = mf->ptr;
        cnt = 0;
 
        if (regexp) {
@@ -197,29 +195,53 @@ static unsigned int contains(struct diff_filespec *one, struct diff_options *o,
                        cnt++;
                }
        }
-       diff_free_filespec_data(one);
        return cnt;
 }
 
 static int has_changes(struct diff_filepair *p, struct diff_options *o,
                       regex_t *regexp, kwset_t kws)
 {
+       struct userdiff_driver *textconv_one = get_textconv(p->one);
+       struct userdiff_driver *textconv_two = get_textconv(p->two);
+       mmfile_t mf1, mf2;
+       int ret;
+
        if (!o->pickaxe[0])
                return 0;
 
-       if (!DIFF_FILE_VALID(p->one)) {
-               if (!DIFF_FILE_VALID(p->two))
-                       return 0; /* ignore unmerged */
+       /*
+        * If we have an unmodified pair, we know that the count will be the
+        * same and don't even have to load the blobs. Unless textconv is in
+        * play, _and_ we are using two different textconv filters (e.g.,
+        * because a pair is an exact rename with different textconv attributes
+        * for each side, which might generate different content).
+        */
+       if (textconv_one == textconv_two && diff_unmodified_pair(p))
+               return 0;
+
+       fill_one(p->one, &mf1, &textconv_one);
+       fill_one(p->two, &mf2, &textconv_two);
+
+       if (!mf1.ptr) {
+               if (!mf2.ptr)
+                       ret = 0; /* ignore unmerged */
                /* created */
-               return contains(p->two, o, regexp, kws) != 0;
-       }
-       if (!DIFF_FILE_VALID(p->two))
-               return contains(p->one, o, regexp, kws) != 0;
-       if (!diff_unmodified_pair(p)) {
-               return contains(p->one, o, regexp, kws) !=
-                      contains(p->two, o, regexp, kws);
+               ret = contains(&mf2, o, regexp, kws) != 0;
        }
-       return 0;
+       else if (!mf2.ptr) /* removed */
+               ret = contains(&mf1, o, regexp, kws) != 0;
+       else
+               ret = contains(&mf1, o, regexp, kws) !=
+                     contains(&mf2, o, regexp, kws);
+
+       if (textconv_one)
+               free(mf1.ptr);
+       if (textconv_two)
+               free(mf2.ptr);
+       diff_free_filespec_data(p->one);
+       diff_free_filespec_data(p->two);
+
+       return ret;
 }
 
 static void diffcore_pickaxe_count(struct diff_options *o)

diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 461d27ac202cc51f1f09b7a18de8b747198700e4..53ec330ce8a09cfde5b3b5e7661bb6dd457ba0db 100755 (executable)
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -96,6 +96,18 @@ test_expect_success 'grep-diff (-G) operates on textconv data (modification)' '
        test_cmp expect actual
 '
 
+test_expect_success 'pickaxe (-S) operates on textconv data (add)' '
+       echo one >expect &&
+       git log --root --format=%s -S0 >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'pickaxe (-S) operates on textconv data (modification)' '
+       echo two >expect &&
+       git log --root --format=%s -S1 >actual &&
+       test_cmp expect actual
+'
+
 cat >expect.stat <<'EOF'
  file | Bin 2 -> 4 bytes
  1 file changed, 0 insertions(+), 0 deletions(-)