Junio C Hamano <gitster@xxxxxxxxx> writes: > 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. 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. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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