From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Tim Henigan <tim.henigan@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH v3] diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes
Date: Wed, 20 Jun 2012 12:21:12 -0700 [thread overview]
Message-ID: <7vipelvlg7.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120620185237.GA31520@sigill.intra.peff.net> (Jeff King's message of "Wed, 20 Jun 2012 14:52:37 -0400")
Jeff King <peff@peff.net> 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".
next prev parent reply other threads:[~2012-06-20 19:21 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-18 19:28 [PATCH v3] diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes Tim Henigan
2012-06-18 19:45 ` Jeff King
2012-06-18 20:09 ` Junio C Hamano
2012-06-19 13:05 ` Tim Henigan
2012-06-19 13:58 ` Jeff King
2012-06-19 16:47 ` Tim Henigan
2012-06-20 13:38 ` Tim Henigan
2012-06-20 16:06 ` Jeff King
2012-06-20 18:44 ` Junio C Hamano
2012-06-20 18:52 ` Jeff King
2012-06-20 19:21 ` Junio C Hamano [this message]
2012-06-21 15:07 ` Tim Henigan
2012-06-21 16:55 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vipelvlg7.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=tim.henigan@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.