[0/3] format-patch: --interiff/--range-diff tweaks

Message

Eric Sunshine Sept. 8, 2020, 7:16 a.m. UTC

This series addresses a few comments[2,3,4,5] which cropped up during
review of the series which added --interdiff and --range-diff options to
git-format-patch[1]. That series made it into 'next' before I could address
the comments, so these patches (based upon 'master') make minor tweaks
"incrementally" (over 2 years late).

[1]: https://lore.kernel.org/git/[e-mail]/
[2]: https://lore.kernel.org/git/CACsJy8C8RK6HkfoEYJGZg=sgtJS0WksHD3=7Souw3jYebRo=[e-mail]/
[3]: https://lore.kernel.org/git/[e-mail]/
[4]: https://lore.kernel.org/git/CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=[e-mail]/
[5]: https://lore.kernel.org/git/[e-mail]/

Eric Sunshine (3):
  diff: move show_interdiff() from its own file to diff-lib
  diff-lib: tighten show_interdiff()'s interface
  format-patch: use 'origin' as start of current-series-range when known

 Makefile      |  1 -
 builtin/log.c | 22 +++++++++++++---------
 diff-lib.c    | 25 +++++++++++++++++++++++++
 diff.h        |  7 +++++++
 interdiff.c   | 28 ----------------------------
 interdiff.h   |  8 --------
 log-tree.c    |  4 ++--
 7 files changed, 47 insertions(+), 48 deletions(-)
 delete mode 100644 interdiff.c
 delete mode 100644 interdiff.h

Comments

Junio C Hamano Sept. 8, 2020, 10:10 p.m. UTC | #1

Eric Sunshine <[e-mail]> writes:

> This series addresses a few comments[2,3,4,5] which cropped up during
> review of the series which added --interdiff and --range-diff options to
> git-format-patch[1]. That series made it into 'next' before I could address
> the comments, so these patches (based upon 'master') make minor tweaks
> "incrementally" (over 2 years late).

The last step subtly changes the behaviour, if I am reading its
description correctly.  Does it deserve a documentation update, or
are we just making the code behave "better" but still within the
boundary of how it is documented to work, hence no need to update
the doc (but deserves an advertisement in the release notes)?

Thanks.

Eric Sunshine Sept. 9, 2020, 6:02 a.m. UTC | #2

On Tue, Sep 8, 2020 at 6:11 PM Junio C Hamano <[e-mail]> wrote:
> Eric Sunshine <[e-mail]> writes:
> > This series addresses a few comments[2,3,4,5] which cropped up during
> > review of the series which added --interdiff and --range-diff options to
> > git-format-patch[1]. That series made it into 'next' before I could address
> > the comments, so these patches (based upon 'master') make minor tweaks
> > "incrementally" (over 2 years late).
>
> The last step subtly changes the behaviour, if I am reading its
> description correctly.  Does it deserve a documentation update, or
> are we just making the code behave "better" but still within the
> boundary of how it is documented to work, hence no need to update
> the doc (but deserves an advertisement in the release notes)?

I honestly don't have an answer because I have trouble reasoning about
these cases (perhaps due to unclear mental model). That's part of the
reason why this patch series took so long. The first two patches were
ready in September 2018, but I kept putting off the third one because
I was having trouble understanding your suggested changes. It only
started to click the other day when I sat down and really studied your
proposal for a long time. Attempting to write a meaningful commit
message, rather than a hand-wavy one, forced me to think about it even
more critically, which helped (perhaps even more than the code itself)
to better understand it.

I wouldn't be opposed to dropping the last patch, however, with
whatever understanding I gained, I do agree that the way you suggested
coding it does make more sense and is more intuitive. But that doesn't
mean that it is sufficiently concrete in my brain to say whether a
documentation update is warranted and, if so, exactly how to
articulate such an update.