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>

Ævar Arnfjörð Bjarmason committed Apr 12, 2021 at 19:15 UTC 03c1f14acf5169d8e6c06a60c28ee5a6cfb9fd54
1 file changed +21 -2
diffcore-pickaxe.c
+21 -2
@@ -228,6 +228,7 @@ void diffcore_pickaxe(struct diff_options *o)
228 int opts = o->pickaxe_opts;
229 regex_t regex, *regexp = NULL;
230 kwset_t kws = NULL;
231 + pickaxe_fn fn;
232
233 if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
234 int cflags = REG_EXTENDED | REG_NEWLINE;
@@ -235,6 +236,20 @@ void diffcore_pickaxe(struct diff_options *o)
236 cflags |= REG_ICASE;
237 regcomp_or_die(&regex, needle, cflags);
238 regexp = &regex;
239 +
240 + if (opts & DIFF_PICKAXE_KIND_G)
241 + fn = diff_grep;
242 + else if (opts & DIFF_PICKAXE_REGEX)
243 + fn = has_changes;
244 + else
245 + /*
246 + * We don't need to check the combination of
247 + * -G and --pickaxe-regex, by the time we get
248 + * here diff.c has already died if they're
249 + * combined. See the usage tests in
250 + * t4209-log-pickaxe.sh.
251 + */
252 + BUG("unreachable");
253 } else if (opts & DIFF_PICKAXE_KIND_S) {
254 if (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE &&
255 has_non_ascii(needle)) {
@@ -251,10 +266,14 @@ void diffcore_pickaxe(struct diff_options *o)
266 kwsincr(kws, needle, strlen(needle));
267 kwsprep(kws);
268 }
269 + fn = has_changes;
270 + } else if (opts & DIFF_PICKAXE_KIND_OBJFIND) {
271 + fn = NULL;
272 + } else {
273 + BUG("unknown pickaxe_opts flag");
274 }
275
256 - pickaxe(&diff_queued_diff, o, regexp, kws,
257 - (opts & DIFF_PICKAXE_KIND_G) ? diff_grep : has_changes);
276 + pickaxe(&diff_queued_diff, o, regexp, kws, fn);
277
278 if (regexp)
279 regfree(regexp);