git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Jeff KingNext: Jeff King
Message 17 of 21 in “[GSOC] ref-filter: use single strbuf for all output”
  1. [GSOC] ref-filter: use single strbuf for all outputZheNing Hu via GitGitGadget, Apr 5, 2021
  2. Eric SunshineApr 5, 2021
  3. ZheNing HuApr 6, 2021
  4. Derrick StoleeApr 5, 2021
  5. ZheNing HuApr 6, 2021
  6. Jeff KingApr 5, 2021
  7. ZheNing HuApr 6, 2021
  8. ZheNing HuApr 6, 2021
  9. Jeff KingApr 6, 2021
  10. ZheNing HuApr 6, 2021
  11. René ScharfeApr 6, 2021
  12. ZheNing HuApr 7, 2021
  13. [GSOC] ref-filter: use single strbuf for all outputZheNing Hu via GitGitGadget, Apr 7, 2021
  14. Junio C HamanoApr 7, 2021
  15. ZheNing HuApr 8, 2021
  16. Jeff KingApr 7, 2021
  17. ZheNing HuApr 8, 2021
  18. Jeff KingApr 8, 2021
  19. ZheNing HuApr 8, 2021
  20. Jeff KingApr 8, 2021
  21. ZheNing HuApr 8, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.