pickaxe: refactor function selection in diffcore-pickaxe()

It's hard to read this codepath at a glance and reason about exactly
what combination of -G and -S will compile either regexes or kwset,
and whether we'll then dispatch to "diff_grep" or "has_changes".

Then in the "--find-object" case we aren't using the callback
function, but were previously passing down "has_changes".

Refactor this code to exhaustively check "opts", it's now more obvious
what callback function (or none) we want under what mode.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index a278b9b71d9b58af197269cc6340050bba98acbe..953b6ec1b4aff8450ca7556390fd314805063d42 100644 (file)
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -228,6 +228,7 @@ void diffcore_pickaxe(struct diff_options *o)
        int opts = o->pickaxe_opts;
        regex_t regex, *regexp = NULL;
        kwset_t kws = NULL;
+       pickaxe_fn fn;
 
        if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
                int cflags = REG_EXTENDED | REG_NEWLINE;
@@ -235,6 +236,20 @@ void diffcore_pickaxe(struct diff_options *o)
                        cflags |= REG_ICASE;
                regcomp_or_die(&regex, needle, cflags);
                regexp = &regex;
+
+               if (opts & DIFF_PICKAXE_KIND_G)
+                       fn = diff_grep;
+               else if (opts & DIFF_PICKAXE_REGEX)
+                       fn = has_changes;
+               else
+                       /*
+                        * We don't need to check the combination of
+                        * -G and --pickaxe-regex, by the time we get
+                        * here diff.c has already died if they're
+                        * combined. See the usage tests in
+                        * t4209-log-pickaxe.sh.
+                        */
+                       BUG("unreachable");
        } else if (opts & DIFF_PICKAXE_KIND_S) {
                if (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE &&
                    has_non_ascii(needle)) {
@@ -251,10 +266,14 @@ void diffcore_pickaxe(struct diff_options *o)
                        kwsincr(kws, needle, strlen(needle));
                        kwsprep(kws);
                }
+               fn = has_changes;
+       } else if (opts & DIFF_PICKAXE_KIND_OBJFIND) {
+               fn = NULL;
+       } else {
+               BUG("unknown pickaxe_opts flag");
        }
 
-       pickaxe(&diff_queued_diff, o, regexp, kws,
-               (opts & DIFF_PICKAXE_KIND_G) ? diff_grep : has_changes);
+       pickaxe(&diff_queued_diff, o, regexp, kws, fn);
 
        if (regexp)
                regfree(regexp);