Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@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.
>
> For an arbitrary %(modifier), I'd agree, but in the particular case of
> %(padright), I think it only makes sense if applied to something of
> variable length
>

>> 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)'
>
> Yes, but on the other hand we already have:
>
>   git log --format='%<|(50)A very long irrevlevancy|%an|'
>
> that pads/truncate %an. So, consistancy would dictate that Karthik's
> version is the right one.

>
>> 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)".
>
> I think there are valid argument for the 3 versions. I'm fine with any
> of them, as long as there's a test that shows which one is picked.
>