Re: [PATCH 03/18] branch-diff: first rudimentary implementation
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- May 4, 2018, 07:03 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1805040900570.77@tvgsbejvaqbjf.bet>
- In-Reply-To
- <CAPig+cSvHWvb0dsGkjL69yzbBvgaT7oJm6nFuGWeA6Jw0NpYUw@mail.gmail.com>
Hi Eric,
On Thu, 3 May 2018, Eric Sunshine wrote:
Show 16 quoted lines
> On Thu, May 3, 2018 at 11:30 AM, Johannes Schindelin > <johannes.schindelin@gmx.de> wrote: > > At this stage, `git branch-diff` can determine corresponding commits of > > two related commit ranges. This makes use of the recently introduced > > implementation of the Hungarian algorithm. > > > > The core of this patch is a straight port of the ideas of tbdiff, the > > seemingly dormant project at https://github.com/trast/tbdiff. > > > > The output does not at all match `tbdiff`'s output yet, as this patch > > really concentrates on getting the patch matching part right. > > > > Note: due to differences in the diff algorithm (`tbdiff` uses the > > Pythong module `difflib`, Git uses its xdiff fork), the cost matrix > > s/Pythong/Python/
Yep!
Show 22 quoted lines
> > calculated by `branch-diff` is different (but very similar) to the one
> > calculated by `tbdiff`. Therefore, it is possible that they find
> > different matching commits in corner cases (e.g. when a patch was split
> > into two patches of roughly equal length).
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > diff --git a/builtin/branch-diff.c b/builtin/branch-diff.c
> > @@ -19,6 +23,279 @@ static int parse_creation_weight(const struct option *opt, const char *arg,
> > +static int read_patches(const char *range, struct string_list *list)
> > +{
> > + [...]
> > + struct strbuf buf = STRBUF_INIT, line = STRBUF_INIT;
> > + [...]
> > + } else if (starts_with(line.buf, " ")) {
> > + strbuf_addbuf(&buf, &line);
> > + strbuf_addch(&buf, '\n');
> > + }
> > +
> > + continue;
>
> Unnecessary blank line above 'continue'?Sure.
Show 11 quoted lines
> > + } else if (starts_with(line.buf, "@@ ")) > > + strbuf_addstr(&buf, "@@"); > > + [...] > > + } > > + fclose(in); > > + > > + if (util) > > + string_list_append(list, buf.buf)->util = util; > > + strbuf_release(&buf); > > strbuf_release(&line);
Yes!
Show 22 quoted lines
> > + if (finish_command(&cp))
> > + return -1;
> > +
> > + return 0;
> > +}
> > @@ -32,9 +309,63 @@ int cmd_branch_diff(int argc, const char **argv, const char *prefix)
> > + if (argc == 2) {
> > + if (!strstr(argv[0], ".."))
> > + warning(_("no .. in range: '%s'"), argv[0]);
> > + strbuf_addstr(&range1, argv[0]);
> > +
> > + if (!strstr(argv[1], ".."))
> > + warning(_("no .. in range: '%s'"), argv[1]);
> > + strbuf_addstr(&range2, argv[1]);
> > + } else if (argc == 1) {
> > + if (!b)
> > + die(_("single arg format requires a symmetric range"));
> > + } else {
> > + error("Need two commit ranges");
>
> Other warning/error messages emitted by this function are not
> capitalized: s/Need/need/Right. And it is also not translated. Fixed both.
Thank you for helping me make this patch series better!
Ciao, Dscho