Re: [PATCH v3 09/20] range-diff: adjust the output of the commit pairs

> On Tue, Jul 3, 2018 at 7:26 AM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > This change brings `git range-diff` yet another step closer to
> > feature parity with tbdiff: it now shows the oneline, too, and indicates
> > with `=` when the commits have identical diffs.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > diff --git a/range-diff.c b/range-diff.c
> > @@ -251,9 +253,57 @@ static void get_correspondences(struct string_list *a, struct string_list *b,
> > +static void output_pair_header(struct strbuf *buf,
> > +                              struct patch_util *a_util,
> > +                              struct patch_util *b_util)
> >  {
> > -       return find_unique_abbrev(&util->oid, DEFAULT_ABBREV);
> > +       static char *dashes;
> > +       struct object_id *oid = a_util ? &a_util->oid : &b_util->oid;
> > +       struct commit *commit;
> > +
> > +       if (!dashes) {
> > +               char *p;
> > +
> > +               dashes = xstrdup(find_unique_abbrev(oid, DEFAULT_ABBREV));
> 
> It's nice to see that the bulk of the range-diff functionality has
> been libified in this re-roll (residing in range-diff.c rather than

> builtin/range-diff.c as in earlier versions), so it's somewhat
> surprising to see libified code holding onto the 'dashes' buffer like
> this in a static variable. An alternative would have been for the
> caller to pass in the same buffer to output_pair_header() for re-use,
> and then dispose of it at the end of processing.

> 
> > +               for (p = dashes; *p; p++)
> > +                       *p = '-';
> > +       }
> > +
> > +       strbuf_reset(buf);
> 
> ...much like 'buf' is allocated by the caller, passed in and re-used
> for each invocation, then released by the caller at the end.