xdiff-interface: allow early return from xdiff_emit_line_fn

Finish the change started in the preceding commit and allow an early
return from "xdiff_emit_line_fn" callbacks, this will allows
diffcore-pickaxe.c to save itself redundant work.

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.

There was some work in this area of xdiff-interface.[ch] recently with
3b40a090fd4 (diff: avoid generating unused hunk header lines,
2018-11-02) and 7c61e25fbf1 (diff: use hunk callback for word-diff,
2018-11-02).

In combination those two changes allow us to not do any work on the
hunks and diff at all, but didn't change the status quo with regards
to consumers that e.g. want the diff lines, but might want to abort
early.

Whereas now we can abort e.g. on the first "-line" of a 1000 line diff
if that's all we needed.

This interface is rather scary as noted in the comment to
xdiff-interface.h being added here, as noted there a future change
could add more exit codes, and hack xdl_emit_diff() and friends to
ignore or skip things more selectively as a result.

I did not see an inherent reason for why xdl_emit_{diffrec,record}()
could not be changed to ferry the "xdiff_emit_line_fn" error code
upwards instead of returning -1 on all "ret < 0".

But doing so would require corresponding changes in xdl_emit_diff(),
xdl_diff(). I didn't see any issue with narrowly doing that to
accomplish what I needed here, but it would leave xdiff's own return
values in an inconsistent state.

Instead I've left it at returning a more conventional (for git's own
codebase) 1 for an early return, and translating it (or rather, all
non-zero) to -1 for xdiff's consumption.

The reason for most of the "stop" complexity in xdiff_outf() is
because we want to be able to abort early, but do so in a way that
doesn't skip the appropriate strbuf_reset() invocations.

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

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 5d8c8c67dc2b22cbddb8a4149e649f786f7ff4bc..50c0ef759dd9e08acbc80e152555ed03609b8374 100644 (file)
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -37,9 +37,12 @@ static int consume_one(void *priv_, char *s, unsigned long size)
        char *ep;
        while (size) {
                unsigned long this_size;
+               int ret;
                ep = memchr(s, '\n', size);
                this_size = (ep == NULL) ? size : (ep - s + 1);
-               priv->line_fn(priv->consume_callback_data, s, this_size);
+               ret = priv->line_fn(priv->consume_callback_data, s, this_size);
+               if (ret)
+                       return ret;
                size -= this_size;
                s += this_size;
        }
@@ -50,11 +53,14 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 {
        struct xdiff_emit_state *priv = priv_;
        int i;
+       int stop = 0;
 
        if (!priv->line_fn)
                return 0;
 
        for (i = 0; i < nbuf; i++) {
+               if (stop)
+                       return 1;
                if (mb[i].ptr[mb[i].size-1] != '\n') {
                        /* Incomplete line */
                        strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size);
@@ -63,17 +69,21 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 
                /* we have a complete line */
                if (!priv->remainder.len) {
-                       consume_one(priv, mb[i].ptr, mb[i].size);
+                       stop = consume_one(priv, mb[i].ptr, mb[i].size);
                        continue;
                }
                strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size);
-               consume_one(priv, priv->remainder.buf, priv->remainder.len);
+               stop = consume_one(priv, priv->remainder.buf, priv->remainder.len);
                strbuf_reset(&priv->remainder);
        }
+       if (stop)
+               return -1;
        if (priv->remainder.len) {
-               consume_one(priv, priv->remainder.buf, priv->remainder.len);
+               stop = consume_one(priv, priv->remainder.buf, priv->remainder.len);
                strbuf_reset(&priv->remainder);
        }
+       if (stop)
+               return -1;
        return 0;
 }
 

diff --git a/xdiff-interface.h b/xdiff-interface.h
index 0198f9632f5022d833dbda7a4a6f1d8a53165635..7d1724abb646839304504d67c4fd172ce66ad5e2 100644 (file)
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -11,6 +11,23 @@
  */
 #define MAX_XDIFF_SIZE (1024UL * 1024 * 1023)
 
+/**
+ * The `xdiff_emit_line_fn` function can return 1 to abort early, or 0
+ * to continue processing. Note that doing so is an all-or-nothing
+ * affair, as returning 1 will return all the way to the top-level,
+ * e.g. the xdi_diff_outf() call to generate the diff.
+ *
+ * Thus returning 1 means you won't be getting any more diff lines. If
+ * you need something in-between those two options you'll to use
+ * `xdl_emit_hunk_consume_func_t` and implement your own version of
+ * xdl_emit_diff().
+ *
+ * We may extend the interface in the future to understand other more
+ * granular return values. While you should return 1 to exit early,
+ * doing so will currently make your early return indistinguishable
+ * from an error internal to xdiff, xdiff itself will see that
+ * non-zero return and translate it to -1.
+ */
 typedef int (*xdiff_emit_line_fn)(void *, char *, unsigned long);
 typedef void (*xdiff_emit_hunk_fn)(void *data,
                                   long old_begin, long old_nr,