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日周四 下午10:51写道：
>
> On Thu, Apr 08, 2021 at 10:43:54PM +0800, ZheNing Hu wrote:
>
> > > What I meant was that we should get rid of show_ref_array_items(), as
> > > well, and just use format_ref_array_item() everywhere. This whole
> > > wrapper is only saving us a few lines, and it makes it harder to see
> > > what the function is doing. Likewise for pretty-print ref. But I dunno.
> > > Maybe that is all going too far.
> > >
> >
> > Ok... so you mean we just use a loop like in branch.c, and get rid of
> > show_ref_array_items() and show_ref_array_item().
> > (We can still use the optimization of reuse bufs)
>
> Yes, something like this:
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index cb9c81a046..55297fe297 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -22,6 +22,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>         struct ref_array array;
>         struct ref_filter filter;
>         struct ref_format format = REF_FORMAT_INIT;
> +       struct strbuf output = STRBUF_INIT;
> +       struct strbuf err = STRBUF_INIT;
>
>         struct option opts[] = {
>                 OPT_BIT('s', "shell", &format.quote_style,
> @@ -80,8 +82,16 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>
>         if (!maxcount || array.nr < maxcount)
>                 maxcount = array.nr;
> -       for (i = 0; i < maxcount; i++)
> -               show_ref_array_item(array.items[i], &format);
> +
> +       for (i = 0; i < maxcount; i++) {
> +               strbuf_reset(&output);
> +               if (format_ref_array_item(array.items[i], &format, &output, &err))
> +                       die("%s", err.buf);
> +               fwrite(output.buf, 1, output.len, stdout);
> +               putchar('\n');
> +       }
> +
> +       strbuf_release(&output);
>         ref_array_clear(&array);
>         return 0;
>  }
>
> It is dropping a few lines by assuming that the error buf is only
> touched when we return an error (which IMHO is a reasonable assumption
> to make).
>

I agree, err buffer does not need to be reused.

> -Peff

Thanks.
--
ZheNing Hu