Fix issue with formatting multiple trailer keys

Fix issue with formatting multiple trailer keys

[Brooke Kuhlmann]

Hello. 👋

I'm seeing strange issues with log/listing trailer information for commits and tags and I think I'm experiencing a bug where when I attempt to list commits and tags with specific trailer keys, I get new lines showing up in my output.

Here's my steps to recreate using a simple Bash script:

``` 
mkdir demo
cd demo
git init

touch one.txt
git add .
git commit --message "Added one" --trailer Milestone=patch --trailer Issue=111
git tag 0.0.0 --message "Version 0.0.0" --no-sign --trailer Files:1 --trailer Duration:1

touch two.txt
git add .
git commit --message "Added two" --trailer Milestone=patch --trailer Issue=222
git tag 0.0.1 --message "Version 0.0.1" --no-sign --trailer Files:1 --trailer Duration:1
```

Notice that I'm using trailers for my commits *and* tags. Here's what my git log looks like:

``` 
git log --pretty=format:"%h %an% %s %d %cr. %(trailers:key=Milestone) %(trailers:key=Issue)"
```

The above produces this output:

08a07b717bef Brooke KuhlmannAdded two  (HEAD -> main, tag: 0.0.1) 5 minutes ago. Milestone: patch
 Issue: 222

01c4c182c85e Brooke KuhlmannAdded one  (tag: 0.0.0) 5 minutes ago. Milestone: patch
 Issue: 111

...and if I list my tags using this command:

``` 
git tag --list \
        --format="%(refname:short) %(taggerdate:short) %(taggername) %(subject) %(trailers:key=Files) %(trailers:key=Duration)"
```

...I get this output:

0.0.0 2024-09-06 Brooke Kuhlmann Version 0.0.0 Files: 1
Duration: 1
 Files: 1
Duration: 1

0.0.1 2024-09-06 Brooke Kuhlmann Version 0.0.1 Files: 1
Duration: 1
 Files: 1
Duration: 1

Notice, when logging trailer information for commits and tags, I get unexpected new line characters showing up in the output. I expect to see all information printed on the same line without any new lines showing up.

Also, I want to point out that when listing tag trailers, I get the "Duration" key showing up twice. I'm not sure if that's related or not but seems like very weird behavior.

Re: Fix issue with formatting multiple trailer keys

[Jeff King]

On Fri, Sep 06, 2024 at 01:55:46PM -0600, Brooke Kuhlmann wrote:

> ``` 
> git tag --list \
>         --format="%(refname:short) %(taggerdate:short) %(taggername) %(subject) %(trailers:key=Files) %(trailers:key=Duration)"
> ```
> 
> ...I get this output:
> 
> 0.0.0 2024-09-06 Brooke Kuhlmann Version 0.0.0 Files: 1
> Duration: 1
>  Files: 1
> Duration: 1
> 
> 0.0.1 2024-09-06 Brooke Kuhlmann Version 0.0.1 Files: 1
> Duration: 1
>  Files: 1
> Duration: 1
> 
> Notice, when logging trailer information for commits and tags, I get
> unexpected new line characters showing up in the output. I expect to
> see all information printed on the same line without any new lines
> showing up.
> 
> Also, I want to point out that when listing tag trailers, I get the
> "Duration" key showing up twice. I'm not sure if that's related or not
> but seems like very weird behavior.

There are two things going on here:

  1. Some of the trailer options are stored in a global, rather than a
     per-placeholder basis. This includes the list of keys to show. So
     you basically end up with a filter list that includes _both_ keys,
     and that list is used for both %(trailers) items.

     IMHO this is a bug, and I have a patch (which I'll send along with
     the one I mentioned in the other thread) to fix it.

     In the meantime you can work around it by specifying both keys in
     a single invocation, like:

       %(trailers:key=Files,key=Duration)

     Obviously that doesn't work if you don't want them adjacent in the
     final output.

  2. By default, each trailer is terminated with a newline. But you can
     specify your own separator, like so:

       %(trailers:key=Files,key=Duration,separator= )

     See the documentation for the trailers placeholder in "git help
     log". Depending on your trailers, "valueonly" might also be useful
     for this kind of formatting.

-Peff

Re: Fix issue with formatting multiple trailer keys

[Brooke Kuhlmann]

Use of a custom separator is a nifty trick. Thanks. A little unintuitive but works. Definitely would be neat if you patch this to work without the custom separator, though.

Thanks for the help. Will look forward to picking up the next patched version of Git.

> On Sep 6, 2024, at 4:34 PM, Jeff King <peff@peff.net> wrote:
> 
> On Fri, Sep 06, 2024 at 01:55:46PM -0600, Brooke Kuhlmann wrote:
> 
>> ``` 
>> git tag --list \
>>        --format="%(refname:short) %(taggerdate:short) %(taggername) %(subject) %(trailers:key=Files) %(trailers:key=Duration)"
>> ```
>> 
>> ...I get this output:
>> 
>> 0.0.0 2024-09-06 Brooke Kuhlmann Version 0.0.0 Files: 1
>> Duration: 1
>> Files: 1
>> Duration: 1
>> 
>> 0.0.1 2024-09-06 Brooke Kuhlmann Version 0.0.1 Files: 1
>> Duration: 1
>> Files: 1
>> Duration: 1
>> 
>> Notice, when logging trailer information for commits and tags, I get
>> unexpected new line characters showing up in the output. I expect to
>> see all information printed on the same line without any new lines
>> showing up.
>> 
>> Also, I want to point out that when listing tag trailers, I get the
>> "Duration" key showing up twice. I'm not sure if that's related or not
>> but seems like very weird behavior.
> 
> There are two things going on here:
> 
>  1. Some of the trailer options are stored in a global, rather than a
>     per-placeholder basis. This includes the list of keys to show. So
>     you basically end up with a filter list that includes _both_ keys,
>     and that list is used for both %(trailers) items.
> 
>     IMHO this is a bug, and I have a patch (which I'll send along with
>     the one I mentioned in the other thread) to fix it.
> 
>     In the meantime you can work around it by specifying both keys in
>     a single invocation, like:
> 
>       %(trailers:key=Files,key=Duration)
> 
>     Obviously that doesn't work if you don't want them adjacent in the
>     final output.
> 
>  2. By default, each trailer is terminated with a newline. But you can
>     specify your own separator, like so:
> 
>       %(trailers:key=Files,key=Duration,separator= )
> 
>     See the documentation for the trailers placeholder in "git help
>     log". Depending on your trailers, "valueonly" might also be useful
>     for this kind of formatting.
> 
> -Peff

Re: Fix issue with formatting multiple trailer keys

[Jeff King]

On Fri, Sep 06, 2024 at 05:09:08PM -0600, Brooke Kuhlmann wrote:

> Use of a custom separator is a nifty trick. Thanks. A little
> unintuitive but works. Definitely would be neat if you patch this to
> work without the custom separator, though.

The separator will always be required (even if you use multiple
%(trailer) blocks), because it overrides the default of terminating with
a newline. And we can't switch that default without breaking
compatibility for existing users.

So I think after my series you'll have:

  %(trailers:key=Files,separator=) %(trailers:key=Duration,separator=)

or similar.

-Peff

Re: Fix issue with formatting multiple trailer keys

[Brooke Kuhlmann]

OK, thanks. Yeah, that backwards compatibility stipulation is a bit rough in this case but makes sense. Your patch does mean that you could potentially use different separators for each key which might be useful. The other nice thing about your patch is you'll be able to easily apply different colors to each key if desired so that's a nice win too.

For the moment, in case it helps, I ended up crafting a Bash function for local use that looks like this;

----- SNIPPET-----
# Label: Git Tag List
# Description: List tags in tabular form.
gtl() {
  local format="\
%(color:yellow)%(refname:short)%(color:reset)|%(taggerdate:short)\
|%(color:blue)%(color:bold)%(taggername)%(color:reset)|%(subject)\
|%(color:green)%(trailers:key=Commits,key=Files,key=Deletions,\
key=Insertions,key=Duration,separator=|)%(color:reset)"

  git tag --list --color --format="$format" | column -s "|" -t
}
----- SNIPPET-----

This allows me to list all the keys I need and use a pipe as a separator for column formatting. I think that's quite nice while not making the format string anymore complex than it needs to be.


> On Sep 9, 2024, at 5:03 PM, Jeff King <peff@peff.net> wrote:
> 
> On Fri, Sep 06, 2024 at 05:09:08PM -0600, Brooke Kuhlmann wrote:
> 
>> Use of a custom separator is a nifty trick. Thanks. A little
>> unintuitive but works. Definitely would be neat if you patch this to
>> work without the custom separator, though.
> 
> The separator will always be required (even if you use multiple
> %(trailer) blocks), because it overrides the default of terminating with
> a newline. And we can't switch that default without breaking
> compatibility for existing users.
> 
> So I think after my series you'll have:
> 
>  %(trailers:key=Files,separator=) %(trailers:key=Duration,separator=)
> 
> or similar.
> 
> -Peff