Re: [PATCH v2] [GSOC] ref-filter: use single strbuf for all output

[ZheNing Hu <adlternative@gmail.com>]

Jeff King <peff@peff.net> 于2021年4月8日周四 上午5:27写道：
>
> 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.

> 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