Re: [PATCH v2 3/4] rebase: fix garbled progress display with '-x'

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 12, 2019, 16:21 UTC
Message-ID: <xmqqimtakc4x.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <xmqqy327kfw1.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 22 quoted lines

> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> Make sure that the previously displayed "Rebasing (N/M)" line is
>> cleared by using the term_clear_line() helper function added in the
>> previous patch.
>>
>> A couple of other rebase commands print similar messages, e.g.
>> "Stopped at <abbrev-oid>... <subject>" for the 'edit' or 'break'
>> commands, or the "Successfully rebased and updated <full-ref>." at the
>> very end.  These are so long that they practically always overwrite
>> that "Rebasing (n/m)" progress line, but let's be prudent, and clear
>> the last line before printing these, too.
>> ...
>> Note that this patch doesn't completely eliminate the possibility of
>> similar garbled outputs, ...
>> too soon, and it would either flicker or be invisible.
>
> The user of term_clear_line() in this patch always guard themselves
> behind "we do not do this if we are producing verbose output," but
> the proposed log message does not explain why they need to do so.
>
> Is it because it is so obvious to potential readers?

Answering myself, it is due to this part in sequencer.c:

   3721                                 if (!opts->quiet)
   3722                                         fprintf(stderr, "Rebasing (%d/%d)%s",
   3723                                                 todo_list->done_nr,
   3724                                                 todo_list->total_nr,
   3725                                                 opts->verbose ? "\n" : "\r");

That is, under 'verbose' mode, we do not try to reuse a single line to show different steps of rebase in the progress output, but consume one line per one step, so it would not be necessary to erase what is previously written on the line.

I do not think it is so obvious, though.