Hi Eric,
On Mon, 16 Jul 2018, Eric Sunshine wrote:
> On Tue, Jul 3, 2018 at 7:26 AM Johannes Schindelin via GitGitGadget
> <gitgitgadget@xxxxxxxxx> 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@xxxxxx>
> > ---
> > 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
Can we *please* stop calling it "re-roll"? Thanks.
(Or are you really "never gonna give you up, never gonna let you down"?)
> 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.
Sure, to be honest, I had completely forgotten about what I did there, and
had to read up on it to fix it.
>
> > + 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.
Yep, I now pass in another strbuf, `dashes`.
Thanks,
Dscho
