Jeff King <peff@xxxxxxxx> writes:
> I thought this bug would be enough to show that diffopt.found_changes is
> not clear enough. It is the source of the original bug (the code should
> have been using HAS_CHANGES instead of found_changes), and it gave at
> least one of the bug investigators (i.e., me) quite a bit of confusion
> to understand why found_changes was not being set when diff_flush found
> changes.
I think when found_changes was introduced so that diff can indicate
more than what HAS_CHANGES (i.e. there is a blob-level difference
exists) can represent, the patch forgot to update the no-index
codepath.
> IOW, as a naive reader of the "struct diff_options", how do I understand
> the difference between HAS_CHANGES and found_changes?
HAS_CHANGES and found_changes should be implementation detail of
diff_result_code() and as long as we do not add outside users of it,
the names should not matter too much. If we were to rename them,
HAS_CHANGES should also be made more descriptive to hint what it
means ("object level difference exists"), I would think. Given the
recent discussion on "diff/log -L <bottom>,<top>", found_changes
would mean "content level change that the caller cares about
exists".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html