Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option

[Karthik Nayak <karthik.188@gmail.com>]

On Sun, Jul 12, 2015 at 7:17 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>
> I guess if you can have multiple arguments after ':' in an atom, then
> you have wiggle room for future. But it looks like you only accept one
> argument after ':'.. (I only checked the version on 'pu'). Having an
> "alignment atom" to augment the real one (like %< changes the behavior
> of the next placeholder), could also work, but it adds dependency
> between atoms, something I don't think ref-filter.c is ready for.
>

I was thinking of something on the lines of having a function which right
before printing checks if any "align" option is given to the end of a given
item and aligns it accordingly, this ensures that any item which needs to
have such an option can easily do so.

https://github.com/KarthikNayak/git/commit/0284320483d6442a6425fc665e740f9f975654a1

This is what I came up with, you could have a look and let me know if
you have any
suggestions.

> Another thing, the atom value is also used for sorting. When used for
> sorting, I think these padding spaces should not be generated or it
> may confuse the sort algorithm. Left alignment may be ok, right or
> center alignment (in future?), not  so much. Perhaps we should do the
> padding in a separate phase, outside populate_value(). If you go this
> route, having separate atoms for alignment works better: you don't
> have to parse them in populate_value() which is for actual values, and
> you can handle dependency easily (I think).

This was cleared out in your other reply.

>
> By the way, please consider adding _() back to translatable strings,
> usually those die() or warn(), or "[ahead %s]"... In the last case,
> because you don't really know how long the string is after
> translation, avoid hard coding buffer size (to 40).

Yes, sure, maybe a cleanup patch at the end to do all of cleanup any
such use cases :)

Thanks for all the suggestions.

-- 
Regards,
Karthik Nayak