Solve a long-standing item for "git log -Grx" of us e.g. finding "+ str" in the diff context and noting that we had a "hit", but xdiff diligently continuing to generate and spew the rest of the diff at us. The TODO item has been there since "git log -G" was implemented. See f506b8e8b5f (git log/diff: add -G<regexp> that greps in the patch text, 2010-08-23). Our xdiff interface also had the limitation of not being able to abort early since the beginning, see d9ea73e0564 (combine-diff: refactor built-in xdiff interface., 2006-04-05). Although at that time "xdiff_emit_line_fn" was called "xdiff_emit_consume_fn", and "xdiff_emit_hunk_fn" didn't exist yet. But now with the support added in the preceding ""xdiff-interface: allow early return from xdiff_emit_{line,hunk}_fn" commit we can return early, and furthermore test the functionality of the new early-exit xdiff-interface by having a BUG() call here to die if it ever starts handing us needless work again. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> --- diffcore-pickaxe.c | 31 ++++++++++++++++++++----------- xdiff-interface.h | 5 +++++ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index 94601072bde..f11b38b7121 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -27,13 +27,12 @@ static int diffgrep_consume(void *priv, char *line, unsigned long len) if (line[0] != '+' && line[0] != '-') return 0; if (data->hit) - /* - * NEEDSWORK: we should have a way to terminate the - * caller early. - */ - return 0; - data->hit = !regexec_buf(data->regexp, line + 1, len - 1, 1, - ®match, 0); + BUG("Already matched in diffgrep_consume! Broken xdiff_emit_line_fn?"); + if (!regexec_buf(data->regexp, line + 1, len - 1, 1, + ®match, 0)) { + data->hit = 1; + return -1; + } return 0; } @@ -45,6 +44,7 @@ static int diff_grep(mmfile_t *one, mmfile_t *two, struct diffgrep_cb ecbdata; xpparam_t xpp; xdemitconf_t xecfg; + int ret; if (!one) return !regexec_buf(regexp, two->ptr, two->size, @@ -63,10 +63,19 @@ static int diff_grep(mmfile_t *one, mmfile_t *two, ecbdata.hit = 0; xecfg.ctxlen = o->context; xecfg.interhunkctxlen = o->interhunkcontext; - if (xdi_diff_outf(one, two, discard_hunk_line, diffgrep_consume, - &ecbdata, &xpp, &xecfg)) - return 0; - return ecbdata.hit; + + /* + * An xdiff error might be our "data->hit" from above. See the + * comment for xdiff_emit_{line,hunk}_fn in xdiff-interface.h + * for why. + */ + ret = xdi_diff_outf(one, two, discard_hunk_line, diffgrep_consume, + &ecbdata, &xpp, &xecfg); + if (ecbdata.hit) + return 1; + if (ret) + return ret; + return 0; } static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws, diff --git a/xdiff-interface.h b/xdiff-interface.h index 1b27d6104ce..347d8a4425f 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -25,6 +25,11 @@ * granular return values, but for now use it carefully, or consider * e.g. using discard_hunk_line() if you say just don't care about * hunk headers. + * + * Note that just returning -1 will make your early return + * indistinguishable from an error internal to xdiff. See "diff_grep" + * in diffcore-pickaxe.c for a trick to work around this, i.e. using + * the "consume_callback_data" to note the desired early return. */ typedef int (*xdiff_emit_line_fn)(void *, char *, unsigned long); typedef int (*xdiff_emit_hunk_fn)(void *data, -- 2.30.0.284.gd98b1dd5eaa7