Re: [PATCH v2 00/14] output improvements for git range-diff

> Hi Thomas,
> 
> On Fri, 5 Jul 2019, Thomas Gummerer wrote:
> 
> > It's been quite a while since I sent the RFC [1] (thanks all for the
> > comments on that), and the series changed shapes quite a bit since the
> > last round.
> >
> > Since it's been such a long time, just to remind everyone, the goal of
> > this series is to make the range-diff output clearer, by showing
> > information about the filenames to which the current diff belongs.
> 
> Thank you for that reminder ;-)
> 
> > In the previous round, we did this using "section headers" that
> > include information about the current file and adding that to the
> > outer diff's hunk headers.
> >
> > In this round we still keep the section headers (with slightly more
> > information), but in addition we also add the filename to the inner
> > diff hunk headers.  In the outer diff hunk headers we then display
> > either the section header or the inner diff hunk header using a
> > userdiff pattern.
> 
> 
> I like this idea!
> 
> > In terms of code changes the biggest change is that we're now re-using
> > the 'parse_git_header' function from the apply code to parse the diff
> > headers, instead of trying to parse them with some hacky parsing code
> > in range-diff.c.  This way we are sure that the diff headers are
> > properly parsed.
> 
> Yep, very good.
> 
> > I had also considered just outputting the section headers directly
> > from 'git log', but then decided against that.  Parsing the headers
> > allows a future enhancement of range-diff, where we would allow
> > parsing an mbox file [2].
> 
> Thanks you for your consideration; I still would like to have the option
> at some stage to compare a patch series from public-inbox.org/git to the
> commits in `pu`, without having to fiddle with finding a valid base commit
> to apply the patches on.

> > I split the "only pass required data" commits up, in the hopes of
> > making them easier to review, but I'm also happy to squash them if
> > people feel like that makes it easier to review them.
> 
> I found it very easy to review in the current form, thank you for putting
> in the extra effort.
> 
> > An added advantage of this is that we're also getting rid of things
> > like the similarity index, which are not important in the range-diff,
> > and are thus not represented in the "section header".
> >
> > One thing that did not change is that the new/deleted strings are not
> > translated in this version either.  This is similar to the regular
> > diff output, where we also don't translate these.  We can still
> > consider translating them in the future though.
> >
> > [1]: https://public-inbox.org/git/20190414210933.20875-1-t.gummerer@gmail.com/
> > [2]: https://github.com/gitgitgadget/git/issues/207
> >
> > I'm including the range-diff between this version of the series and
> > the RFC, and a diff between the range diff and the range-diff without
> > these changes below.  Probably not useful in reviewing the code, but
> > good to show off the changes made in this series.
> 
> Indeed!
> 
> I very much like the idea, and the current iteration. I offered a couple
> of minor suggestions, in the hope that you find them helpful.