On Fri, Aug 7, 2015 at 9:34 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Wed, Aug 5, 2015 at 2:54 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote: >> Implement an `align` atom which will act as a modifier atom and align >> any string with or without an %(atom) appearing before a %(end) atom >> to the right, left or middle. >> >> It is followed by `:<type>,<paddinglength>`, where the `<type>` is >> either left, right or middle and `<paddinglength>` is the total length >> of the padding to be performed. If the atom length is more than the >> padding length then no padding is performed. e.g. to pad a succeeding >> atom to the middle with a total padding size of 40 we can do a >> --format="%(align:middle,40).." >> >> Add documentation and tests for the same. > > I forgot to mention in my earlier review of this patch that you should > explain in the commit message, and probably the documentation, this > this implementation (assuming I'm understanding the code) does not > correctly support nested %(foo)...%(end) constructs, where %(foo) > might be %(if:), %(truncate:), %(cut:), or even a nested %(align:), or > some as yet unimagined modifier. Supporting nesting of these > constructs will require pushing the formatting states onto a stack (or > invoking the parser recursively). > >> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx> Good point, I have been working on this parallely and it works for now, I'll send that with the %(if) and %(end) feature. But for now, it should be documented and added in the commit message. Using a linked list of sorts where whenever a new modifier atom is encountered a new state is created, and once %(end) is encountered we can pop that state into the previous state. -- 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