Re: [PATCH v2] [GSOC] ref-filter: use single strbuf for all output
- From
ZheNing Hu <adlternative@gmail.com>
- Date
- Apr 8, 2021, 12:18 UTC
- Message-ID
- <CAOLTT8QvdLeWz=cDOoVFV8Lrk2QL2wf_jwDc6oK5j+6gup+Png@mail.gmail.com>
- In-Reply-To
- <YG4jxKQ3z1R+8Jfz@coredump.intra.peff.net>
Jeff King <peff@peff.net> 于2021年4月8日周四 上午5:27写道:
Show 40 quoted lines
>
> On Wed, Apr 07, 2021 at 03:26:48PM +0000, ZheNing Hu via GitGitGadget wrote:
>
> > diff --git a/ref-filter.c b/ref-filter.c
> > index f0bd32f71416..27bbf9b6c8ac 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -2435,6 +2435,26 @@ int format_ref_array_item(struct ref_array_item *info,
> > return 0;
> > }
> >
> > +void show_ref_array_items(struct ref_array_item **info,
> > + const struct ref_format *format,
> > + size_t n)
> > +{
> > + struct strbuf final_buf = STRBUF_INIT;
> > + struct strbuf error_buf = STRBUF_INIT;
> > + size_t i;
> > +
> > + for (i = 0; i < n; i++) {
> > + if (format_ref_array_item(info[i], format, &final_buf, &error_buf))
> > + die("%s", error_buf.buf);
> > + fwrite(final_buf.buf, 1, final_buf.len, stdout);
> > + strbuf_reset(&error_buf);
> > + strbuf_reset(&final_buf);
> > + putchar('\n');
> > + }
> > + strbuf_release(&error_buf);
> > + strbuf_release(&final_buf);
> > +}
>
> I think this is a reasonable direction to take the solution: wrapping
> the loop so that the reuse of the buffers can be included there.
>
> But I do wonder if we should go the opposite direction, and get rid of
> show_ref_array_item() entirely. It only has two callers, both of which
> could just write the loop themselves. That is more code, but perhaps it
> would make it more clear what is going on in those callers, and to give
> them more flexibility.
>Indeed. I think `pretty_print_ref()` is proof that we may need to keep `show_ref_array_item()` because If it modified to `show_ref_array_items(...,1);` it will look very strange.
Show 7 quoted lines
> I notice there's a third user of the ref-filter.c code in > builtin/branch.c that does not use show_ref_array_item(). Instead, it > loops itself and calls format_ref_array_item(). I think this is because > it is sometimes columnizing the results. Perhaps git-tag and > for-each-ref would want to learn the same trick, in which case they'd be > happy to have the open-coded loop. >
Yes, a special judgment about colopts is used in the `print_ref_list()` of `branch.c`.
> Either way, it probably makes sense to introduce the same optimization > to the case in builtin/branch.c. >
I agree in `branch.c` here can learn this reusing bufs optimization.
> -Peff
Thanks. -- ZheNing Hu