On Tue, Jul 28, 2015 at 7:05 PM, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> This version also doesn't use the printing options provided by >> branch.c. > > Do you mean "provided by ref-filter.{c,h}"? > Yes! my bad. >> 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? Absolutely 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). > Nope! This is exactly what I'm trying :D > 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) > This I gotta try! Have been just keeping the flow the same and trying to mess around with how formatting works instead. > 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". This could work for refname, as each ref_array_item holds the refname, But other atoms are only filled in after a call to verify_ref_format(). What we could do would be after filling in all atom values, have a loop through all items with their atom values and maybe implement this. But I don't see this as priority for now, so will mark it off for later. Thanks -- 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