All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Anders Waldenborg <anders@0x63.nu>
Cc: Christian Couder <christian.couder@gmail.com>,
	Junio C Hamano <gitster@pobox.com>, git <git@vger.kernel.org>,
	Jonathan Tan <jonathantanmy@google.com>
Subject: Re: Questions about trailer configuration semantics
Date: Mon, 27 Jul 2020 19:42:17 -0400	[thread overview]
Message-ID: <20200727234217.GA802697@coredump.intra.peff.net> (raw)
In-Reply-To: <875za8r2fu.fsf@0x63.nu>

On Tue, Jul 28, 2020 at 12:57:51AM +0200, Anders Waldenborg wrote:

> >> > > 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.
> 
> Right, and it also happens with "--only-input" (part of "--parse")
> 
> "--only-input" is documented as follows:
> 
>    Output only trailers that exist in the input; do not add any from the
>    command-line or by following configured trailer.* rules.

I think I meant there only that we wouldn't add new trailers (e.g., from
"trailers.*.ifMissing"). But I do agree that it might be simpler if we
can just say "we don't look at trailer.<token>.* config at all in
--only-input mode. I _think_ that's true (we probably do look at
trailer.separators, but the rest of the token-specific ones look like
they're all about writing or modifying, not reading).

> > 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.
> 
> Would that option also be set for the parsing done by "%(trailers)"
> pretty format specifier?

I thnk %(trailers) isn't quite the same as "--parse", because you have
to say "only" or "unfold" yourself. But yeah, that option should
certainly be available there, if not the 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.
> 
> I'm also mainly using in reading trailers (mostly with pretty format
> "%(trailers:key=x)") and then these convenience shortcuts doesn't really
> seem helpful, they rather add a small risk of mangling my data. Not that
> this has caused any problems for me in practice.

Yeah, pondering it a bit more, I think trailer.<token>.* doesn't really
make any sense for any reading operation (including %(trailers) or
--parse). I guess it _could_ be useful to normalize names in some
instances, but it's as likely to confuse or break somebody as to help.

-Peff

  reply	other threads:[~2020-07-27 23:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-27 16:45 Questions about trailer configuration semantics Anders Waldenborg
2020-07-27 17:18 ` Junio C Hamano
2020-07-27 18:37   ` Christian Couder
2020-07-27 19:40     ` Jeff King
2020-07-27 22:57       ` Anders Waldenborg
2020-07-27 23:42         ` Jeff King [this message]
2020-07-27 20:11     ` Junio C Hamano
2020-07-27 22:17       ` Anders Waldenborg
2020-07-27 23:05         ` Junio C Hamano
2020-07-28  0:01           ` Anders Waldenborg
2020-07-27 21:41     ` Anders Waldenborg
2020-07-27 22:53       ` Junio C Hamano
2020-07-27 23:17         ` Anders Waldenborg
2020-07-28  7:07       ` Christian Couder
2020-07-28 15:41         ` Jeff King
2020-07-28 16:40           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200727234217.GA802697@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=anders@0x63.nu \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.