Re: [PATCH v2 08/10] tag.c: use 'ref-filter' APIs

On Sun, Jul 12, 2015 at 3:15 PM, Duy Nguyen [off-list ref] wrote:

On Thu, Jul 9, 2015 at 5:58 PM, Karthik Nayak [off-list ref] wrote:

quoted

-static int show_reference(const char *refname, const struct object_id *oid,
-                         int flag, void *cb_data)
-{

...

quoted

-
-       if (match_pattern(filter->name_patterns, refname)) {

....

quoted

-               printf("%-15s ", refname);
-               show_tag_lines(oid, filter->lines);
-               putchar('\n');
-       }
-
-       return 0;
-}

...

quoted

+       if (filter->lines)
+               format = "%(refname:shortalign=16)";
+       else
+               format = "%(refname:short)";

I can see this is a faithful conversion, but this looks line an
opportunity to avoid this special limit 15/16. Even on git.git "git
tag -l -n1" already breaks alignment with *.msysgit.* tags (ok maybe
msysgit, not purely git.git) When you get to "branch -l", it
calculates the max-width automatically so you probably need
"%(refname:shortalign)" any way. "shortalign" (i.e. create the "align"
version for every modifier) does not look good because it could double
the number of modifiers and let's not thinking about truncation
options or right alignment..

What I was thinking of was getting rid of the whole "align" feature where
you provide a value to which it would align.

Something like:  --format="%(item:modifieralign)" which would use something
on the lines of what the max-width calculator in branch -l uses, to get the max
alignment size. But the problem is that ref-filter goes through the refs using
a function which has no connections with the atoms used. So a more practical
solution would be --format="%(item:modifieralign=X)" where we could provide a
means of calculating X via ref-filter. Something like this in tag.c:

int max_width = get_max_width("<item to get max_width of>");
use this max_width to then do a
--format="%(item:modifieralign=X)", where X = max_width

What do you think?

-- 
Regards,
Karthik Nayak