Matthieu Moy <matthieu....@grenoble-inp.fr> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to