[1/4] sideband: don't lose clear-to-eol at packet boundary

Commit Message

Jiang Xin June 17, 2021, 3:17 a.m. UTC

When "demultiplex_sideband()" sees a nonempty message ending with CR or
LF on the sideband #2, it adds "suffix" string to clear to the end of
the current line, which helps when relaying a progress display whose
records are terminated with CRs.  But if it sees a single LF, no
clear-to-end suffix should be appended, because this single LF is used
to end the progress display by moving to the next line, and the final
progress display above should be preserved.

However, the code forgot that depending on the length of the payload
line, such a CR may fall exactly at the packet boundary and the
number of bytes before the CR from the beginning of the packet could
be zero.  In such a case, the message that was terminated by the CR
were leftover in the "scratch" buffer in the previous call to the
function and we still need to clear to the end of the current line.

Helped-by: Junio C Hamano <[e-mail]>
Helped-by: Nicolas Pitre <[e-mail]>
Signed-off-by: Jiang Xin <[e-mail]>
---
 sideband.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Patch

diff --git a/sideband.c b/sideband.c
index 6f9e026732..85bddfdcd4 100644
--- a/sideband.c
+++ b/sideband.c
@@ -183,8 +183,31 @@  int demultiplex_sideband(const char *me, int status,
 		while ((brk = strpbrk(b, "\n\r"))) {
 			int linelen = brk - b;
 
+			/*
+			 * For message accross packet boundary, there would have
+			 * a nonempty "scratch" buffer from last call of this
+			 * function, and there may have a leading CR/LF in "buf".
+			 * For this case we should add a clear-to-eol suffix to
+			 * clean leftover letters we previously have written on
+			 * the same line.
+			 */
+			if (scratch->len && !linelen)
+				strbuf_addstr(scratch, suffix);
+
 			if (!scratch->len)
 				strbuf_addstr(scratch, DISPLAY_PREFIX);
+
+			/*
+			 * A use case that we should not add clear-to-eol suffix
+			 * to empty lines:
+			 *
+			 * For progress reporting we may receive a bunch of
+			 * percentage updates followed by '\r' to remain on the
+			 * same line, and at the end receive a single '\n' to
+			 * move to the next line. We should preserve the final
+			 * status report line by not appending clear-to-eol
+			 * suffix to this single line break.
+			 */
 			if (linelen > 0) {
 				maybe_colorize_sideband(scratch, b, linelen);
 				strbuf_addstr(scratch, suffix);