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

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

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

Hi,

On Tue, 30 Apr 2019, SZEDER Gábor wrote:

> When running a command with the 'exec' instruction during an
> interactive rebase session, or for a range of commits using 'git
> rebase -x', the output can be a bit garbled when the name of the
> command is short enough:
>
>   $ git rebase -x true HEAD~5
>   Executing: true
>   Executing: true
>   Executing: true
>   Executing: true
>   Executing: true)
>   Successfully rebased and updated refs/heads/master.
>
> Note the ')' at the end of the last line.  It gets more garbled as the
> range of commits increases:
>
>   $ git rebase -x true HEAD~50
>   Executing: true)
>   [ repeated 3 more times ]
>   Executing: true0)
>   [ repeated 44 more times ]
>   Executing: true00)
>   Successfully rebased and updated refs/heads/master.
>
> Those extra numbers and ')' are remnants of the previously displayed
> "Rebasing (N/M)" progress lines that are usually completely
> overwritten by the "Executing: <cmd>" lines, unless 'cmd' is short and
> the "N/M" part is long.
>
> Make sure that the previously displayed "Rebasing (N/M)" line is
> completely covered up by printing a terminal width worth of space
> characters.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>

Makes sense.

> This issue has already been present in the scripted rebase as well.
>
> As far as I could tell, if any other rebase instruction prints a
> message, then that tends to be so long (including abbreviated commit
> OIDs and whatnot) that they practically always overwrite that
> "Rebasing (N/M)" progress line (well, except, perhaps, when rebasing
> billions of commits at a time?).
>
>  sequencer.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> 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.

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

Ciao,
Dscho

>  			*end_of_arg = '\0';
>  			res = do_exec(r, arg);
>  			*end_of_arg = saved;
> --
> 2.21.0.1181.g24122a4251
>
>