Re: [RFC/PATCH] Port branch.c to use ref-filter APIs
From: Matthieu Moy <hidden>
Date: 2016-06-15 23:05:54
Karthik Nayak [off-list ref] writes:
This version also doesn't use the printing options provided by branch.c.
Do you mean "provided by ref-filter.{c,h}"?
I wanted to discuss how exactly to go about that, because in branch.c, we might need to change the --format based on attributes such as abbrev, verbose and so on. But ref-filter expects us to verify the format before filtering.
I took time to understand the problem, but here's my understanding: ref-filter expects the code to look like format = ...; verify_ref_format(format); filter_refs(...); for (...) show_ref_array_item(..., format, ...); and in the case of "git branch -v", you need to align the sha1s based on the longest branch name, i.e. use %(padright:N+1) where N is the longest branch name. And you can get N only after calling filter_refs, while you need it for verify_ref_format(). Is my understanding correct? If so, what prevents swapping the order of verify_ref_format and filter_refs? I understand that verify_ref_format() builds used_atom and other data-structures, hence it has to be called before show_ref_array_item() and before sorting, but I don't think you need it before filter_refs (I may have missed something though). So, the code could look like: filter_refs(...) if (--format is given) format = the argument of --format else format = STRBUF_INIT; strbuf_add(&format, "%(some_directive_to_display '*' if needed)"); if (verbose) strbuf_addf(&format, "%(padright:%d)", max_width); ... verify_ref_format(format); ref_array_sort(...); for (...) show_ref_array_item(...); (BTW, a trivial helper function to display the whole ref_array could help. It would avoid having each caller write the 'for' loop) Ideally, you could also have a modifier atom %(alignleft) that would not need an argument and that would go through the ref_array to find the maxwidth, and do the alignment. That would give even more flexibility to the end users of "for-each-ref --format". -- Matthieu Moy http://www-verimag.imag.fr/~moy/