Junio C Hamano [off-list ref] writes:

The code looks much easier to reason about than the previous rounds.

A few comments about the design.

- Are there other possible values that might fit in this "optimize"
  member, and what kind of behaviour would they trigger, that we
  can envision?  I do not think of any and that is why the "enum
  diff_optimize" member in the diff_options structure smells more
  like a "bool dry_run".

I was thinking about DIFF_OPT_BUFFER , but "bool dry_run" would be good
enough for now.

  By the way, giving a member "diff_" prefix when the enclosing
  struct is clearly about "diff" by having a name "diff_options" is
  often a waste of readers' time.

Good advice, field name should avoid overlapping with struct name.

- It is unclear why the dry-run need to imply 0-line context.

I was thinking about matching ignored regex in `quick_consume()`, in which
case we only need to match regex for changes rather than context, so I set
ctxlen to zero. But anyway more `if` reduce both readability and maintainability,
so I won’t set ctxlen in the next version.

- diff_flush_patch_quietly() would be a better name for
  diff_flush_patch_quiet().

Understand, I suppose the idea behind this is to stick to proper grammar as
much as possible when choosing names.

On to the details.

quoted

diff --git a/diff.c b/diff.c
index dca87e164f..5254ef9373 100644
--- a/diff.c
+++ b/diff.c

@@ -2444,6 +2444,15 @@ static int fn_out_consume(void *priv, char *line, unsigned long len)

return 0;
}

+static int quick_consume(void *priv, char *line, unsigned long len)
+{
+ struct emit_callback *ecbdata = priv;
+ struct diff_options *o = ecbdata->opt;
+
+ o->found_changes = 1;
+ return 1;
+}

OK, as a non-zero return value from consume callbacks is supposed
to signal an error and causes xdiff_outf() an early return, this
serves as a short-cut.  One downside is that we cannot truly notice
and signal an error to our callers, as we will see in a later hunk.

I want to think about how to solve this problem later, but I believe I shouldn’t
address it in this patch.

Thanks,
Lidong