Re: [PATCH] pretty: Add %(trailer:X) to display single trailer

[Jeff King <peff@peff.net>]

On Mon, Oct 29, 2018 at 06:05:34PM +0100, Anders Waldenborg wrote:

> 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).

IMHO that is probably an acceptable tradeoff. We haven't really made any
rules for quoting arbitrary values in other %() sequences. I think it's
something we may want to have eventually, but as long as the rule for
now is "you can't do that", I think it would be OK to loosen it later.

> > 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)"

Yeah, I was thinking that "(" was taken as a special character, but I
guess immediately followed by ")" it is easy to parse left-to-right with
no ambiguity.

Would it include the leading space, too? It would be nice if it could be
combined with "% " in an orthogonal way. I guess in theory "% ()D" would
work, but it may need some tweaks to the parsing.

> 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.

Yeah, I agree we should be able to sanitize anything. It's not strictly
related to your patch, though, so you may or may not want to go down
this rabbit hole. :)

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

In theory, yeah. I agree it's getting a bit magical.

> 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.

Yes. I had dreams of eventually stuffing all of those as options into
the placeholders themselves. So "%s" would eventually have a long-form
of "%(subject)", and in that syntax it could be:

  %(subject:lpad=10,filename)

or something. I'm not completely opposed to:

  %[lpad=10,filename]%(subject)

which keeps the "formatting" arguments out of the regular placeholders.
On the other hand, if the rule were not "this affects the next
placeholder" but had a true ending mark, then we could make a real
parse-tree out of it, and format chunks of placeholders. E.g.:

  %(format:lpad=30,filename)%(subject) %(authordate)%(end)

would pad and format the whole string with two placeholders. I know that
going down this road eventually involves reinventing XML, but I think
having an actual tree structure may not be an unreasonable thing to
shoot for.

I dunno. You certainly don't need to solve all of these issues for what
you want to do. My main concern for now is to avoid introducing new
syntax that we'll be stuck with forever, even though it may later become
redundant (or worse, create parsing ambiguities).

> > 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.

Yes, there's great interest. :)

The formats are not mutually incompatible at this point, so we should be
able to come up with a unified language that maintains backwards
compatibility. One of the tricky parts is that some of the formatters
have more information than others (for-each-ref has a ref, which may
resolve to any object type; cat-file has objects only; --pretty has only
commits).

This was the subject of last year's Outreachy work. There's still a ways
to go, but you can find some of the previous discussions and work by
searching for Olga's work in the archive:

  https://public-inbox.org/git/?q=olga+telezhnaya

I've also cc'd her here, as she's still been doing some work since then.

-Peff