Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes: > See my remark on previous patch: this test is not sufficient. You do > not only need to check that %(padright) is taken into account, but you > also need to check that it is taken into account for the right atom and > only this one. I'd suggest > > --format '%(refname)%(padright:25)|%(refname)|%(refname)|' > > Only the middle column should be padded. This actually raises an interesting point. It is equally valid to view that format string as requesting to pad the first "|" with 24 spaces; in other words, %(padright:N) would apply to the next span, be it a literal string up to the next %(atom), or the %(atom) that comes immediately after it. The thing is, the above _might_ be an OK way to ask the middle refname to be padded, but I somehow find it very unnatural and unintuitive to expect that this: --format '%(padright:25)A Very Long Irrelevancy%(refname)' would do nothing to "A Very Long Irrelevancy" and affects the refname that comes much later in the format string. Or we could simply forbid certain atoms followed by an non-empty literal as an error. My preference between the three is "%(padright:4)", etc. to apply to the "next span", but I can live with "it is an error to pad-right a far-away %(atom)". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html