Re: [PATCH] rebase: fix garbled progress display with '-x'

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2053 bytes --]

Hi,

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
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