On Wed, Jul 25, 2018 at 5:07 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx/
