Re: [PATCH v2 09/18] branch-diff: adjust the output of the commit pairs

From: Elijah Newren <newren@gmail.com>
Date: May 4, 2018, 16:25 UTC
Message-ID: <CABPp-BHbVTmNrZ32ZYQuoH0-XtFL+v7k-J7+t4_JWxds4K1U2Q@mail.gmail.com>
In-Reply-To: <0e4c8279e467e2e75864bcce8ec90cf4f81c2c34.1525448066.git.johannes.schindelin@gmx.de>

Hi Dscho,

On Fri, May 4, 2018 at 8:34 AM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

> This change brings branch-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.
>

<snip>

Show 27 quoted lines

> @@ -270,9 +272,57 @@ static int get_correspondences(struct string_list *a, struct string_list *b,
>         return res;
>  }
>
> -static const char *short_oid(struct patch_util *util)
> +static void output_pair_header(struct strbuf *buf,
> +                              int i, struct patch_util *a_util,
> +                              int j, 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));
> +               for (p = dashes; *p; p++)
> +                       *p = '-';
> +       }
> +
> +       strbuf_reset(buf);
> +       if (i < 0)
> +               strbuf_addf(buf, "-:  %s ", dashes);
> +       else
> +               strbuf_addf(buf, "%d:  %s ", i + 1,

One nice thing tbdiff did was to right align patch numbers (which also helped align other columns in the output). So, for example when there are more than 9 patches I would see output like:

...
 8: a980de43fd =  8: 362ab315ac directory rename detection: testcases
exploring possibly suboptimal merges
 9: 3633e79ed9 =  9: 792e1371d9 directory rename detection:
miscellaneous testcases to complete coverage
10: e10d07ef40 = 10: a0b0a15103 directory rename detection: tests for
handling overwriting untracked files
11: f6d84b503e = 11: a7a436042a directory rename detection: tests for
handling overwriting dirty files
...

whereas branch-diff here is instead giving output of the form

... 8: a980de43fd = 8: 362ab315ac directory rename detection: testcases exploring possibly suboptimal merges 9: 3633e79ed9 = 9: 792e1371d9 directory rename detection: miscellaneous testcases to complete coverage 10: e10d07ef40 = 10: a0b0a15103 directory rename detection: tests for handling overwriting untracked files 11: f6d84b503e = 11: a7a436042a directory rename detection: tests for handling overwriting dirty files ...

Not a critical difference, but it'd be nice to match tbdiff here all the same.

Show 15 quoted lines

> +                           find_unique_abbrev(&a_util->oid, DEFAULT_ABBREV));
> +
> +       if (i < 0)
> +               strbuf_addch(buf, '>');
> +       else if (j < 0)
> +               strbuf_addch(buf, '<');
> +       else if (strcmp(a_util->patch, b_util->patch))
> +               strbuf_addch(buf, '!');
> +       else
> +               strbuf_addch(buf, '=');
> +
> +       if (j < 0)
> +               strbuf_addf(buf, " -:  %s", dashes);
> +       else
> +               strbuf_addf(buf, " %d:  %s", j + 1,

Same comment on these last two strbuf_addf's about alignment.

Elijah