On 04/15, Johannes Schindelin wrote:
> Hi Thomas,
>
> On Sun, 14 Apr 2019, Thomas Gummerer wrote:
>
> > Add the section headers we introduced in the previous commit to the
> > outer diff's hunk headers. This makes it easier to understand which
> > change we are actually looking at. For example an outer hunk header
> > might now look like:
> >
> > @@ -77,15 +78,43 @@ modified file Documentation/config/interactive.txt
> >
> > while previously it would have only been
> >
> > @@ -77,15 +78,43 @@
> >
> > which doesn't give a lot of context for the change that follows.
> >
> > For completeness also add section headers for the commit metadata and
> > the commit message, although they are arguably less important.
> >
> > Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx>
>
> That makes a total lot of sense. So much so that we should protect it by a
> new regression test case.
Yup, definitely. As mentioned in the cover letter, I punted on to
discuss and settle on an output format first, so I don't have to
duplicate a lot of work writing and re-writing the tests ;)
> > diff --git a/range-diff.c b/range-diff.c
> > index aa466060ef..15ac54f369 100644
> > --- a/range-diff.c
> > +++ b/range-diff.c
> > @@ -124,8 +124,10 @@ static int read_patches(const char *range, struct string_list *list)
> > strbuf_addstr(&buf, " ##\n");
> > } else if (in_header) {
> > if (starts_with(line.buf, "Author: ")) {
> > + strbuf_addstr(&buf, " ## Metadata ##\n");
> > strbuf_addbuf(&buf, &line);
> > strbuf_addstr(&buf, "\n\n");
> > + strbuf_addstr(&buf, " ## Commit message ##\n");
>
> Should this not rather be added when `in_header` is set to false?
I don't think so. The commit message counts as part of in_header, and
starts right after the rest of the commit metadata. If we would add
this after when we set in_header to false, we'd write it after the
commit message, which would make the hunk header "@@ ... @@ Metadata",
even when we show the commit message in the output.
> The rest of the patch makes a whole lot of sense to me.
Thanks!
> Thanks,
> Dscho
>
> > } else if (starts_with(line.buf, " ")) {
> > strbuf_rtrim(&line);
> > strbuf_addbuf(&buf, &line);
> > @@ -387,8 +389,8 @@ static void output_pair_header(struct diff_options *diffopt,
> > fwrite(buf->buf, buf->len, 1, diffopt->file);
> > }
> >
> > -static struct userdiff_driver no_func_name = {
> > - .funcname = { "$^", 0 }
> > +static struct userdiff_driver section_headers = {
> > + .funcname = { "^ ## (.*) ##$", REG_EXTENDED }
> > };
> >
> > static struct diff_filespec *get_filespec(const char *name, const char *p)
> > @@ -400,7 +402,7 @@ static struct diff_filespec *get_filespec(const char *name, const char *p)
> > spec->size = strlen(p);
> > spec->should_munmap = 0;
> > spec->is_stdin = 1;
> > - spec->driver = &no_func_name;
> > + spec->driver = §ion_headers;
> >
> > return spec;
> > }
> > --
> > 2.21.0.593.g511ec345e1
> >
> >
