Hi Dscho,

On Fri, May 4, 2018 at 8:34 AM, Johannes Schindelin
<johannes.schinde...@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>
> @@ -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.

> +                           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

Reply via email to