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

Jeff King [off-list ref] writes:

Parsing trailers out of a commit message is _mostly_ easy, but there
area a lot of funny corner cases (e.g., heuristics for how many
non-trailers must be present before a final paragraph isn't a trailer
block anymore).  The code in trailer.c already knows about these corner
cases, but there's no way to access it from the command line.

This series teaches interpret-trailers to parse and output just the
trailers. So now you can do:

  $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |
    git interpret-trailers --parse
  Signed-off-by: Hartmut Henkel [off-list ref]
  Helped-by: Stefan Beller [off-list ref]
  Signed-off-by: Ralf Thielow [off-list ref]
  Acked-by: Matthias Rüster [off-list ref]

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 considered a few different approaches before deciding on
what's here:

  1. The output format is actually the normal "key: value" trailers. I
     considered something more structured like JSON. But the "key:
     value" format is quite easy to parse, once it has been normalized
     (finding the trailers, unfolding whitespace continuation, etc).

  2. This series introduces several orthogonal options which can be used
     together to achieve my goal, when there could just be a "parse"
     mode. Since interpret-trailers is plumbing, I reasoned that the
     individual options might still be useful apart from each other (for
     instance, you could re-normalize existing trailers while writing
     your new ones from a commit hook). I did add a "--parse" for
     convenience and to help point users in the right direction.

     For the same reason (and so we could build on other orthogonal
     features like --in-place and --trim-empty), I decided against
     having a separate command like "git parse-trailers".

  [1/5]: trailer: put process_trailers() options into a struct
  [2/5]: interpret-trailers: add an option to show only the trailers
  [3/5]: interpret-trailers: add an option to show only existing trailers
  [4/5]: interpret-trailers: add an option to normalize output
  [5/5]: interpret-trailers: add --parse convenience option

 Documentation/git-interpret-trailers.txt | 17 ++++++++
 builtin/interpret-trailers.c             | 34 ++++++++++++---
 t/t7513-interpret-trailers.sh            | 73 ++++++++++++++++++++++++++++++++
 trailer.c                                | 65 ++++++++++++++++++++++------
 trailer.h                                | 12 +++++-
 5 files changed, 180 insertions(+), 21 deletions(-)

-Peff

On Wed, Aug 9, 2017 at 10:19 AM, Junio C Hamano [off-list ref] wrote:

Jeff King [off-list ref] writes:

quoted

Parsing trailers out of a commit message is _mostly_ easy, but there
area a lot of funny corner cases (e.g., heuristics for how many
non-trailers must be present before a final paragraph isn't a trailer
block anymore).  The code in trailer.c already knows about these corner
cases, but there's no way to access it from the command line.

This series teaches interpret-trailers to parse and output just the
trailers. So now you can do:

  $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |
    git interpret-trailers --parse
  Signed-off-by: Hartmut Henkel [off-list ref]
  Helped-by: Stefan Beller [off-list ref]
  Signed-off-by: Ralf Thielow [off-list ref]
  Acked-by: Matthias Rüster [off-list ref]

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.

Thanks,
Jake

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

quoted

quoted

  $ git log --format=%B -1 8d44797cc91231cd44955279040dc4a1ee0a797f |
    git interpret-trailers --parse
  Signed-off-by: Hartmut Henkel [off-list ref]
  Helped-by: Stefan Beller [off-list ref]
  Signed-off-by: Ralf Thielow [off-list ref]
  Acked-by: Matthias Rüster [off-list ref]

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