On Mon, Jul 27, 2015 at 9:24 PM, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: > 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 > I agree with Matthieu here, Other than atom values, any user defined string would be of known size, hence padding for such things would make no sense. >> 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. Sorry but I didn't understand what you're trying to say here, Matthieu. > >> 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. > Makes sense, thanks both of you :) -- Regards, Karthik Nayak -- 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