Re: [RFC/PATCH] Port branch.c to use ref-filter APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]