Re: [PATCH v2 05/17] ls-files: buffer full item in strbuf before printing

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Mar 26, 2014 at 9:48 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Buffering so that we can manipulate the strings (e.g. coloring)
> further before finally printing them.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/ls-files.c | 48 +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 47c3880..6e30592 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -47,18 +47,30 @@ static const char *tag_modified = "";
>  static const char *tag_skip_worktree = "";
>  static const char *tag_resolve_undo = "";
>
> -static void write_name(const char *name)
> +static void write_name(struct strbuf *sb, const char *name)
>  {
>         /*
>          * With "--full-name", prefix_len=0; this caller needs to pass
>          * an empty string in that case (a NULL is good for "").
>          */
> -       write_name_quoted_relative(name, prefix_len ? prefix : NULL,
> -                                  stdout, line_terminator);
> +       const char *real_prefix = prefix_len ? prefix : NULL;
> +       if (!line_terminator) {
> +               struct strbuf sb2 = STRBUF_INIT;
> +               strbuf_addstr(sb, relative_path(name, real_prefix, &sb2));
> +               strbuf_release(&sb2);
> +       } else
> +               quote_path_relative(name, real_prefix, sb);
> +       strbuf_addch(sb, line_terminator);
> +}
> +
> +static void strbuf_fputs(struct strbuf *sb, FILE *fp)
> +{
> +       fwrite(sb->buf, sb->len, 1, fp);
>  }
>
>  static void show_dir_entry(const char *tag, struct dir_entry *ent)
>  {
> +       static struct strbuf sb = STRBUF_INIT;
>         int len = max_prefix_len;
>
>         if (len >= ent->len)
> @@ -67,8 +79,10 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
>         if (!dir_path_match(ent, &pathspec, len, ps_matched))
>                 return;
>
> -       fputs(tag, stdout);
> -       write_name(ent->name);
> +       strbuf_reset(&sb);
> +       strbuf_addstr(&sb, tag);
> +       write_name(&sb, ent->name);
> +       strbuf_fputs(&sb, stdout);

strbuf_release(&sb);

>  }
>
>  static void show_other_files(struct dir_struct *dir)
> @@ -134,6 +148,7 @@ static void show_killed_files(struct dir_struct *dir)
>
>  static void show_ce_entry(const char *tag, const struct cache_entry *ce)
>  {
> +       static struct strbuf sb = STRBUF_INIT;
>         int len = max_prefix_len;
>
>         if (len >= ce_namelen(ce))
> @@ -161,16 +176,18 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
>                 tag = alttag;
>         }
>
> +       strbuf_reset(&sb);

'sb' is empty at this point. Why reset it?

>         if (!show_stage) {
> -               fputs(tag, stdout);
> +               strbuf_addstr(&sb, tag);
>         } else {
> -               printf("%s%06o %s %d\t",
> -                      tag,
> -                      ce->ce_mode,
> -                      find_unique_abbrev(ce->sha1,abbrev),
> -                      ce_stage(ce));
> +               strbuf_addf(&sb, "%s%06o %s %d\t",
> +                           tag,
> +                           ce->ce_mode,
> +                           find_unique_abbrev(ce->sha1,abbrev),
> +                           ce_stage(ce));
>         }
> -       write_name(ce->name);
> +       write_name(&sb, ce->name);
> +       strbuf_fputs(&sb, stdout);

strbuf_release(&sb);

>         if (debug_mode) {
>                 const struct stat_data *sd = &ce->ce_stat_data;
>
> @@ -206,7 +223,12 @@ static void show_ru_info(void)
>                         printf("%s%06o %s %d\t", tag_resolve_undo, ui->mode[i],
>                                find_unique_abbrev(ui->sha1[i], abbrev),
>                                i + 1);
> -                       write_name(path);
> +                       /*
> +                        * With "--full-name", prefix_len=0; this caller needs to pass
> +                        * an empty string in that case (a NULL is good for "").
> +                        */
> +                       write_name_quoted_relative(path, prefix_len ? prefix : NULL,
> +                                                  stdout, line_terminator);
>                 }
>         }
>  }
> --
> 1.9.1.345.ga1a145c