On Tue, Dec 18, 2018 at 10:30:04PM +0100, Anders Waldenborg wrote: > > 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?
I think we'd want to move in the direction of enforcing valid expressions for %(foo) placeholders. There's some small value in leaving %X alone if we do not understand "X" (not to mention the backwards %compatibility you mentioned), but I think %() is a pretty deliberate indication that a placeholder was meant there. We already do this for ref-filter expansions: $ git for-each-ref --format='%(foo)' fatal: unknown field name: foo We don't for "--pretty" formats, but I do wonder if anybody would be really mad (after all, we have declared ourselves free to add new placeholders, so such formats are not future-proof). -Peff
