Re: [PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 23, 2018, 19:32 UTC
Message-ID: <CAPig+cS7xLgoL4SAihz0tjZur2c7Ff0YZWAgp0wTv0FCkYGZLQ@mail.gmail.com>
In-Reply-To: <CACsJy8AGwbrEFU2bTynXyQi1DmEYtwHrtmefMz+jD13GQs=9HQ@mail.gmail.com>

On Mon, Jul 23, 2018 at 12:12 PM Duy Nguyen <pclouds@gmail.com> wrote:

Show 12 quoted lines

> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > @@ -215,6 +215,7 @@ struct rev_info {
> >         /* interdiff */
> >         const struct object_id *idiff_oid1;
> >         const struct object_id *idiff_oid2;
> > +       const char *idiff_title;
>
> I feel we're abusing struct rev_info a bit for this since this
> interdiff thing is very builtin/log.c's business and not at all
> related to rev walk. Is it possible (and easy) to just pass
> idfff_title from cmd_format_patch to make_cover_letter()? If it's a
> lot of code, then I guess we can just leave it here.

As originally implemented, this information was passed directly to make_cover_letter(), however, as you discovered in your review of patch 6/14[1], which makes it possible to embed an interdiff in the commentary section of a lone patch, 'struct rev_info' is the only way to pass this information down that deeply in the patch-generation process. So, yes, this pretty much needs to use 'struct rev_info', even if that need doesn't exist at this early step in the series.

[1]: https://public-inbox.org/git/CACsJy8Aw6R8-3kDfhCqunXziajCg9O_1WrEYc4rfKa+-=m1D5g@mail.gmail.com/