Re: [PATCH 5/5] progress: break too long progress bar lines

[Duy Nguyen <pclouds@gmail.com>]

On Mon, Mar 25, 2019 at 5:41 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> Some of the recently added progress indicators have quite long titles,
> which might be even longer when translated to some languages, and when
> they are shown while operating on bigger repositories, then the
> progress bar grows longer than the default 80 column terminal width.
>
> When the progress bar exceeds the width of the terminal it gets
> line-wrapped, and after that the CR at the end doesn't return to the
> beginning of the progress bar, but to the first column of its last
> line.  Consequently, the first line of the previously shown progress
> bar is not overwritten by the next, and we end up with a bunch of
> truncated progress bar lines scrolling past:
>
>   $ LANG=es_ES.UTF-8 git commit-graph write
>   Encontrando commits para commit graph entre los objetos empaquetados:   2% (1599
>   Encontrando commits para commit graph entre los objetos empaquetados:   3% (1975
>   Encontrando commits para commit graph entre los objetos empaquetados:   4% (2633
>   Encontrando commits para commit graph entre los objetos empaquetados:   5% (3292
>   [...]
>
> Prevent this by breaking progress bars after the title once they
> exceed the width of the terminal, so the counter and optional
> percentage and throughput, i.e. all changing parts, are on the last
> line.  Subsequent updates will from then on only refresh the changing
> parts, but not the title, and it will look like this:
>
>   $ LANG=es_ES.UTF-8 ~/src/git/git commit-graph write
>   Encontrando commits para commit graph entre los objetos empaquetados:
>     100% (6584502/6584502), listo.
>   Calculando números de generación de commit graph: 100% (824705/824705), listo.
>   Escribiendo commit graph en 4 pasos: 100% (3298820/3298820), listo.
>
> Note that the number of columns in the terminal is cached by
> term_columns(), so this might not kick in when it should when a
> terminal window is resized while the operation is running.
> Furthermore, this change won't help if the terminal is so narrow that
> the counters don't fit on one line, but I would put this in the "If it
> hurts, don't do it" box.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  progress.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/progress.c b/progress.c
> index b37a5398c5..36aaeea340 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -8,7 +8,7 @@
>   * published by the Free Software Foundation.
>   */
>
> -#include "git-compat-util.h"
> +#include "cache.h"
>  #include "gettext.h"
>  #include "progress.h"
>  #include "strbuf.h"
> @@ -37,6 +37,8 @@ struct progress {
>         struct throughput *throughput;
>         uint64_t start_ns;
>         struct strbuf counters_sb;
> +       int title_len;
> +       int split;
>  };
>
>  static volatile sig_atomic_t progress_update;
> @@ -119,8 +121,22 @@ static void display(struct progress *progress, uint64_t n, const char *done)
>                 const char *eol = done ? done : "\r";
>                 int clear_len = counters_sb->len < last_count_len ?
>                                 last_count_len - counters_sb->len : 0;
> -               fprintf(stderr, "%s: %s%-*s", progress->title,
> -                       counters_sb->buf, clear_len, eol);
> +               int cols = term_columns();
> +
> +               if (progress->split) {
> +                       fprintf(stderr, "  %s%-*s", counters_sb->buf, clear_len,
> +                               eol);
> +               } else if (!done &&
> +                          cols < progress->title_len + counters_sb->len + 2) {
> +                       clear_len = progress->title_len + 1 < cols ?
> +                                   cols - progress->title_len - 1 : 0;
> +                       fprintf(stderr, "%s:%*s\n  %s%s", progress->title,
> +                                       clear_len, "", counters_sb->buf, eol);
> +                       progress->split = 1;
> +               } else {
> +                       fprintf(stderr, "%s: %s%-*s", progress->title,
> +                               counters_sb->buf, clear_len, eol);
> +               }
>                 fflush(stderr);
>                 progress_update = 0;
>         }
> @@ -225,6 +241,8 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,
>         progress->throughput = NULL;
>         progress->start_ns = getnanotime();
>         strbuf_init(&progress->counters_sb, 0);
> +       progress->title_len = strlen(title);

I think you need utf8_strwidth() so that the "cols < title_len"
comparison above works for multibyte encoding too.

> +       progress->split = 0;
>         set_progress_signal();
>         return progress;
>  }
> --
> 2.21.0.539.g07239c3a71.dirty
>


-- 
Duy