Re: [PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Sep 7, 2018, 08:46 UTC
- Message-ID
- <CAPig+cS5VwmNnrpFwpm9Ko2xg3WVsfffDLpkJLtU9ziSFOoyUw@mail.gmail.com>
- In-Reply-To
- <xmqqr2jrt46j.fsf@gitster-ct.c.googlers.com>
On Wed, Jul 25, 2018 at 5:07 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > + if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
> > + struct diff_queue_struct dq;
> > +
> > + memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
> > + DIFF_QUEUE_CLEAR(&diff_queued_diff);
> > +
> > + next_commentary_block(opt, NULL);
> > + fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
> > + show_range_diff(opt->rdiff1, opt->rdiff2,
> > + opt->creation_factor, 1, &opt->diffopt);
> > +
> > + memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
> > + }
> > }
>
> This essentially repeats what is already done for "interdiff".Yes, the two blocks are very similar, although they access different members of 'rev_info' and call different functions to perform the actual diff. I explored ways of avoiding the repeated boilerplate (using macros or passing a function pointer to a driver function containing the boilerplate), but the end result was uglier and harder to understand due to the added abstraction. Introducing next_commentary_block()[1] reduced the repetition a bit.
> Does the global diff_queued_diff gets cleaned up when > show_interdiff() and show_range_diff() return, like diff_flush() > does? Otherwise we'd be leaking the filepairs accumulated in the > diff_queued_diff.
Both show_interdiff() and show_range_diff() call diff_flush(). So, the "temporary" diff_queued_diff set up here does get cleaned up by diff_flush(), as far as I understand (though this is my first foray into the diff-generation code, so I may be missing something). And, the diff_queued_diff which gets "interrupted" by this excursion into interdiff/range-diff gets cleaned up normally when the interrupted diff operation completes.
[1]: https://public-inbox.org/git/20180722095717.17912-6-sunshine@sunshineco.com/