Re: [PATCH] rebase: fix garbled progress display with '-x'
- Date: Fri, 3 May 2019 10:41:01 +0200 (DST)
- From: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
- Subject: Re: [PATCH] rebase: fix garbled progress display with '-x'
On Thu, 2 May 2019, SZEDER Gábor wrote:
> On Tue, Apr 30, 2019 at 06:25:35PM -0400, Johannes Schindelin wrote:
> > On Tue, 30 Apr 2019, SZEDER Gábor wrote:
> > > diff --git a/sequencer.c b/sequencer.c
> > > index 546f281898..c2e4baa90e 100644
> > > --- a/sequencer.c
> > > +++ b/sequencer.c
> > > @@ -3631,6 +3631,12 @@ static int pick_commits(struct repository *r,
> > > int saved = *end_of_arg;
> > > struct stat st;
> > >
> > > + if (!opts->verbose)
> > > + /*
> > > + * Fully cover the previous "Rebasing (n/m)"
> > > + * progress line.
> > > + */
> > > + fprintf(stderr, "%*s\r", term_columns(), "");
> > IIRC there are terminals (`cmd.exe`?) that would advance to the next row
> > automatically when printing the exact number of columns in a row. So this
> > would not work.
> Hrm, I though about using 'term_columns()-1', or moving the '\r' from
> the format string to the string to be printed, but in the end didn't
> do either, because it seemed to work well as it is in the two
> terminals that I tried (on Linux).
> > But isn't there an ANSI sequence that we can use?
> > *clicketyclick*
> > Yes: https://github.com/git/git/blob/v2.21.0/editor.c#L101 (introduced in
> > https://github.com/git/git/commit/abfb04d0c7#diff-cdeec438beb851e450b94a11db9ab7edR89)
> > So maybe we should do the same here, i.e.
> > fputs("\r\033[K", stderr);
> Oh, that would be nice (and not only here, but it could have made the
> changes in 'sg/overlong-progress-fix' a bit simpler as well).
> Unfortunately, however, it only works on non-dumb terminals (note the
> '!is_terminal_dumb()' call in the preceeding condition), while rebase
> hasn't had such a limitation on the terminal yet.
I think it would still be nice to have it in the common case of a capable
Maybe this could even become a helper function, e.g.
`erase_terminal_line_remainder()`? This should also help other developers
discover and use it.