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

[Nicolas Pitre <nico@fluxnic.net>]

On Tue, 15 Jun 2021, Junio C Hamano wrote:

> Nicolas Pitre <nico@fluxnic.net> writes:
> 
> > On Tue, 15 Jun 2021, Jiang Xin wrote:
> >
> >> 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.
> >
> > I'd fix this issue with the following logic:
> >
> > bool pending_clear_to_eol;
> >
> > my_putchar(c) {
> > 	switch (c) {
> > 	case '\r':
> > 	case '\n':
> > 		pending_clear_to_eol = true;
> > 		break;
> > 	default:
> > 		if (pending_clear_to_eol) {
> > 			clear_to_eol();
> > 			pending_clear_to_eol = false;
> > 		}
> > 		break;
> > 	}
> > 	putchar(c);
> > }
> >
> > In other words, you clear the line after printing "remote:" but only if 
> > there is a non \n or \r coming next.
> 
> What puzzles me the most in this discussion is why we do this for
> LF.  I do understand why we need it for CR---the line we are going
> to show message on after emitting CR would be full of leftover
> letters we previously have written before emitting CR, so we'd show
> the message (to overwrite the initial part enough to show our own
> message) and then clear to the end with either ANSI sequence of
> sufficient number of whitespaces.  But line feed would take us to a
> fresh and blank line---there is nothing to clear, no?

Depends. Suppose the local process is doing the progress report with CR.

Then the remote sends a single line with LF.

You expects the remote line to be displayed over the local progress 
report and the local progress report to be resumed on the following 
line. Without the line clearing you might have leftover garbage on the 
remote message line.


Nicolas