Re: [PATCH 0/5] make interpret-trailers useful for parsing

From: Jeff King <peff@peff.net>
Date: Aug 10, 2017, 07:28 UTC
Message-ID: <20170810072822.rj6y6zcqhyfz4yi7@sigill.intra.peff.net>
In-Reply-To: <CA+P7+xoifkJyH34Q0NJdE_=UzWK1SA+2gwyXrHpF7Sv2PBHATQ@mail.gmail.com>

On Thu, Aug 10, 2017 at 12:04:49AM -0700, Jacob Keller wrote:

Show 15 quoted lines

> >>   $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |
> >>     git interpret-trailers --parse
> >>   Signed-off-by: Hartmut Henkel <henkel@vh-s.de>
> >>   Helped-by: Stefan Beller <sbeller@google.com>
> >>   Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
> >>   Acked-by: Matthias Rüster <matthias.ruester@gmail.com>
> >
> > Thank-you, thank-you, thank-you.
> >
> > The above example made me wonder if we also want a format specifier
> > to do the above without piping, but it turns out that we already
> > have "log --format=%(trailers)", so we are good ;-)
> 
> I was going to say, I thought we had a way to get trailers for a
> commit via the pretty format, since that is what i used in the past.

I do like that you could get the trailers for many commits in a single invocation. That doesn't matter for my current use-case, but obviously piping through O(n) interpret-trailers invocations is a bad idea.

But there are a few difficulties with using %(trailers) for this, as it shows everything between the start/end points of the trailer block. In particular:

  1. You don't get any kind of normalization, so you're on your own for
     parsing things like whitespace continuation, extra spaces before
     separators, etc.

  2. It prints non-trailers that fall inside the block. For instance:

       $ git commit --allow-empty -F - <<-\EOF
       subject

       body

       Signed-off-by: me
       this is not a trailer
       Signed-off-by: you
       EOF
 
       $ git log -1 --format=%B | git interpret-trailers --parse
       Signed-off-by: me
       Signed-off-by: you

       $ git log -1 --format='%(trailers)'
       Signed-off-by: me
       this is not a trailer
       Signed-off-by: you

For (1) I think many callers would prefer to see the original formatting. Maybe we'd need a %(trailers:normalize) or something.

I'm tempted to call (2) a bug, but I guess it's unclear whether callers would want to see the whole block, or if they really want just the individual trailers.

-Peff