On Mon, Oct 29, 2018 at 3:14 PM Jeff King [off-list ref] wrote:

Junio's review already covered my biggest question, which is why not
something like "%(trailers:key=ticket)". And likewise making things like
comma-separation options.

Jeff, Junio,

thanks!

Your questions pretty much matches what I (and a colleague I discussed
this with before posting) was concerned about.

My first try actually had it as an option to "trailers". But it got a
bit messy with the argument parsing, and the fact that there was a
fast path making it work when only specified. I did not want to spend
lot of time reworking fixing that before I had some feedback, so I
went for a smallest possible patch to float the idea with (a patch is
worth a 1000 words).

I'll start by reworking my patch to handle %(trailers:key=X)  (I'll
assume keys never contain ')' or ','), and ignore any formatting until
the way forward there is decided (see below).

But my second question is whether we want to provide something more
flexible than the always-parentheses that "%d" provides. That has been a
problem in the past when people want to format the decoration in some
other way.

Maybe just like +/-/space can be used directly after %, a () pair can
be allowed..   E.g "%d" would just be an alias for "%()D",  and for
trailers it would be something like "%()(trailers:key=foo)"

There is another special cased placeholder %f (sanitized subject line,
suitable for a filename). Which also could be changed to be a format
specifiier, allowing sanitize any thing, e.g "%!an" for sanitized
author name.

Is even the linebreak to commaseparation a generic thing?
"% ,()(trailers:key=Ticket)"   it starts go look a bit silly.

Then there are the padding modifiers. %<() %<|(). They operate on next
placeholder. "%<(10)%s" Is that a better syntax?
"%()%(trailers:key=Ticket,comma)"

I can also imagine moving all these modifiers into a generic modifier
syntax in brackets (and keeping old for backwards compat)
%[lpad=10,ltrunc=10]s ==  %<(10,trunc)%s
%[nonempty-prefix="%n"]GS ==  %+GS
%[nonempty-prefix=" (",nonempty-suffix=")"]D ==  %d
Which would mean something like this for tickets thing:
%[nonempty-prefix=" (Tickets:",nonempty-suffix=")",commaseparatelines](trailers:key=Ticket,nokey)
which is kinda verbose.

We have formatting magic for "if this thing is non-empty, then show this
prefix" in the for-each-ref formatter, but I'm not sure that we do in
the commit pretty-printer beyond "% ". I wonder if we could/should add a
a placeholder for "if this thing is non-empty, put in a space and
enclose it in parentheses".

Would there be any interest in consolidating those formatters? Even
though they are totally separate beasts today. I think having all
attributes available on long form (e.g "%(authorname)") in addition to
existing short forms in pretty-formatter would make sense.

 anders