> > What is your plan for this function? Is it envisioned that this > will gain more variations of formatting options over time? > Otherwise it seems misnamed (it is not "assign formatting" but > merely "pad to the right"). > I wanna include an 'ifexists' atom in the future where an user can specify a string format to be printed only if the preceding atom holds a value. e.g. '%(ifexists:refname is %s)%(refname)' So I see that going in here. > I also doubt that the logic is sane. More specifically, what does > that "if (v->s[0])" buy you? Yeah, that's wrong, like Eric pointed out. Should have been "if (v->s[0] == '\0')" > > Your caller is iterating over the elements in a format string, > e.g. 'A %(align:20)%(foo) B %(bar) C', and its caller is iterating > over a list of refs, e.g. 'maint', 'master' branches. With that > format string, as long as %(foo) does not expand to something that > exceeds 20 display places or so, I'd expect literal 'B' for all refs > to align, but I do not think this code gives me that; what happens > if '%(foo)' happens to be an empty string for 'maint' but is a > string, say 'x', for 'master'? Oh yeah! That doesn't work, I have a fix in mind I'll reply to this mail with a fix. -- 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