Re: Questions about trailer configuration semantics

From: Jeff King <peff@peff.net>
Date: Jul 27, 2020, 19:40 UTC
Message-ID: <20200727194036.GA795313@coredump.intra.peff.net>
In-Reply-To: <CAP8UFD1XV_jN10yOc2o4=5PtPcvT-RbxhY1H3swZz2r4g-Uzkw@mail.gmail.com>

On Mon, Jul 27, 2020 at 08:37:26PM +0200, Christian Couder wrote:

Show 13 quoted lines

> > > I noticed some undocumented and (at least to me) surprising behavior in
> > > trailers.c.
> > >
> > > When configuring a value in trailer.<token>.key it causes the trailer to
> > > be normalized to that in "git interpret-trailers --parse".
> > > E.g:
> > >  $ printf '\naCKed: Zz\n' | \
> > >    git -c 'trailer.Acked.key=Acked' interpret-trailers --parse
> > >  will emit: "Acked: Zz"
> 
> Yeah, I think that's nice, as it can make sure that the key appears in
> the same way. It's true that it would be better if it would be
> documented.

I'd note that this also happens without --parse.

Show 13 quoted lines

> > > Then there is the replacement by config "trailer.fix.key=Fixes" which
> > > expands "fix" to "Fixes". This happens when using "--trailer 'fix = 123'"
> > > which seems to be expected and useful behavior (albeit a bit unclear in
> > > documentation). But it also happens when parsing incoming trailers, e.g
> > > with that config
> > >  $ printf "\nFix: 1\n" | git interpret-trailers --parse
> > >  will emit: "Fixes: 1"
> [...]
> > > * Should replacement to what is in .key happen also in --parse mode, or
> > >   only for "--trailer"
> 
> I think it's more consistent if it happens in both --parse and
> --trailer mode. I didn't implement --parse though.

I don't recall being aware of this prefix matching until this thread, so I doubt that the current behavior of --parse was something I tried for intentionally. It's mostly just using the existing code, plus a few extra options (listed in the docs). I'm not opposed to adding an option to do strict matching and/or avoid rewriting, and then possibly adding that into --parse by default.

I don't have much of an opinion on which behavior would be preferred. I've never actually had a use case for configuring trailer.*.key, as I usually am only looking at reading existing trailers to collect stats, etc.

-Peff