Re: [PATCH v3] diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 20, 2012, 19:21 UTC
- Message-ID
- <7vipelvlg7.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120620185237.GA31520@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> 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".