Re: [PATCH] rebase: fix garbled progress display with '-x'
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- May 3, 2019, 08:41 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1905031034400.45@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20190501231640.GD14763@szeder.dev>
Hi,
On Thu, 2 May 2019, SZEDER Gábor wrote:
Show 44 quoted lines
> 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 terminal.
Maybe this could even become a helper function, e.g. `erase_terminal_line_remainder()`? This should also help other developers discover and use it.
Ciao, Dscho