Re: [PATCH v4 00/21] Add `range-diff`, a `tbdiff` lookalike

From: Stefan Beller <sbeller@google.com>
Date: Jul 25, 2018, 17:44 UTC
Message-ID: <CAGZ79kZVQZkriu8d9WwYjm01qwb89ntf2dpK_jxRqDHgL1Eq6Q@mail.gmail.com>
In-Reply-To: <xmqq36w94o87.fsf@gitster-ct.c.googlers.com>

On Mon, Jul 23, 2018 at 2:49 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

>
> Stefan Beller <sbeller@google.com> writes:
>
> > On Sat, Jul 21, 2018 at 3:04 PM Johannes Schindelin via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >
> >> Side note: I work on implementing range-diff not only to make life easier for reviewers who have to suffer through v2, v3, ... of my patch series, but also to verify my changes before submitting a new iteration. And also, maybe even more importantly, I plan to use it to verify my merging-rebases of Git for
> >> Windows (for which I previously used to redirect the pre-rebase/post-rebase diffs vs upstream and then compare them using `git diff --no-index`). And of course any interested person can see what changes were necessary e.g. in the merging-rebase of Git for Windows onto v2.17.0 by running a command like:
> >
> > Thanks for making tools that makes the life of a Git developer easier!
> > (Just filed https://github.com/gitgitgadget/gitgitgadget/issues/26
> > which asks to break lines for this cover letter)
>
> Thanks.  These cover letters are unreadable without W Q
> (gnus-article-fill-long-lines)

While I had some comments on how I dislike some aspects of the implementation, I think it proves its usefulness by my usage, so I would suggest to merge it down to next as-is (and as soon as possible); deferring the issues in the implementation to later.

I found running the range-diff on origin/pu to be a pleasant experience, although that still highlights the issues of in-exact coloring (the colors are chosen by the first two characters of the diff, which leads to mis-coloring of diff headers of the inner diff in the outer diff.

But despite the imperfection, I strongly urge to consider the series as-is as good enough for inclusion.

Thanks, Stefan