Re: [PATCH v2 3/4] sideband: append suffix for message whose CR in next pktline

[Jiang Xin <worldhello.net@gmail.com>]

Nicolas Pitre <nico@fluxnic.net> 于2021年6月15日周二 上午10:11写道：
>
> On Tue, 15 Jun 2021, Jiang Xin wrote:
>
> > Junio C Hamano <gitster@pobox.com> 于2021年6月15日周二 上午9:17写道：
> > >
> > > Jiang Xin <worldhello.net@gmail.com> writes:
> > >
> > > >     /*
> > > >      * Let's insert a suffix to clear the end
> > > >      * of the screen line, but only if current
> > > >      * line data actually contains something.
> > > >      */
> > > >
> > > > So my implementation is to try not to break the original
> > > > implementation, and keep the linelen unchanged.
> > >
> > > I knew what you wanted to do from your code---I am questioning if
> > > that "only when something is there" was really sensible, or if it
> > > was just attracting bugs.
> > >
> >
> > @Nicolas, what's your opinion? Is it ok to add clear-to-eol suffix to
> > each line even empty ones?
>
> That would be the simplest thing to do.
>
> But there must have been a reason for doing it otherwise. I just don't
> remember anymore.
>
> Maybe it had to do with progress reporting that does a bunch of
> percentage updates followed by '\r' to remain on the same line, and at
> the end a single '\n' to move to the next line without erasing the final
> status report line. That would be a case for not clearing empty lines.
>

Thank @Nicolas for helping me understand the story behinds the code.

If there are two sideband #2 packets like this:

    PKTLINE(\2 "<progress-1>" CR "<progress-2>" CR)
    PKTLINE(\2 "<message-3>" LF "<message-4>" LF)

We should append clear-to-eol suffix to "<progress-1>", "<progess-2>"
and "<message-3>" to erase the last message displayed on the same
line.  Even though there is no need to add the clear-to-eol suffix to
"<message-4>", always adding suffix before line breaks (CR or LF) of
nonempty message make it simple to program.

If there are empty messages in sideband #2 packets like this:

    PKTLINE(\2 "<progress-1>" CR LF "<message-2>" LF)
    PKTLINE(\2 "<message-3>" LF)

For the empty message between "<progress-1>" and "<message-2>",
nothing to display and no need to add clear-to-eol suffix.

The issue this patch try to fix is like the following example:

    PKTLINE(\2 "<progress-1>" CR "<progress-2>")
    PKTLINE(\2 CR "<message-3>" LF)

The message "<progress-2>" is displayed without a proper clear-to-eol
suffix, because it's eol (CR) is in another pktline.

Since we can distinguished this case by checking the size of
"scratch", IMHO, it better not add suffix before all line breaks.

--
Jiang Xin