Re: [PATCH v4 2/7] pretty: allow %(trailers) options with explicit value

From: Anders Waldenborg <anders@0x63.nu>
Date: Dec 18, 2018, 21:30 UTC
Message-ID: <87o99iwmjn.fsf@0x63.nu>
In-Reply-To: <xmqqa7ldkbwr.fsf@gitster-ct.c.googlers.com>

Junio C Hamano writes:

> That way, we can handle %(trailers:only=bogo) more sensibly,
> no?  Syntactically we can recognize that the user wanted to give
> 'bogo' as the value to 'only', and say "'bogo' is not a boolean" if
> we did so.

I agree that proper error reporting for the pretty formatting strings would be great. But that would depart from the current extremely crude error handling where incorrect formatting placeholders are just left unexpanded. How would such change in error handling be done safely, wrt backwards compatibility changes?

To get good diagnostics for incorrect formatting strings I think the way forward is to have the formatting strings parsed once into some kind of AST or machine (as also mentioned by Jeff) that is just executed many times, instead of parsed each time like today.

 anders