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@xxxxxxxxx>
---
xdiff-interface.c | 18 ++++++++++++++----
xdiff-interface.h | 17 +++++++++++++++++
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 5d8c8c67dc2..50c0ef759dd 100644
--- 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 0198f9632f5..7d1724abb64 100644
--- 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,
--
2.31.1.639.g3d04783866f
