Merge pull request #5648 from dgkmd/feature/statusbar

ui: don't rewrite unchanged lines in status bar

changelog/unreleased/issue-5562

@@ -0,0 +1,10 @@
+Enhancement: Do not rewrite unchanged lines every frame in status bar
+
+Status bars were entirely rewritten every frame if any of its content has updated.
+This behavior had made any user interaction (such as selection) with status bar
+impossible in certain terminal emulators, even with the unchanged lines.
+Now it writes changed lines only at status bar update, thereby improving 
+user experience in those terminal emulators.
+
+https://github.com/restic/restic/issues/5562
+https://github.com/restic/restic/pull/5648

internal/terminal/terminal_posix.go

@@ -11,6 +11,8 @@ const (
 	PosixControlMoveCursorHome = "\r"
 	// PosixControlMoveCursorUp moves cursor up one line
 	PosixControlMoveCursorUp = "\x1b[1A"
+	// PosixControlMoveCursorDown moves cursor down one line
+	PosixControlMoveCursorDown = "\x1b[1B"
 	// PosixControlClearLine clears the current line
 	PosixControlClearLine = "\x1b[2K"
 )
@@ -36,3 +38,14 @@ func PosixMoveCursorUp(wr io.Writer, _ uintptr, n int) error {
 	}
 	return nil
 }
+
+// PosixMoveCursorDown moves the cursor to the line n lines below the current one.
+func PosixMoveCursorDown(wr io.Writer, _ uintptr, n int) error {
+	data := []byte(PosixControlMoveCursorHome)
+	data = append(data, bytes.Repeat([]byte(PosixControlMoveCursorDown), n)...)
+	_, err := wr.Write(data)
+	if err != nil {
+		return fmt.Errorf("write failed: %w", err)
+	}
+	return nil
+}

internal/terminal/terminal_unix.go

@@ -20,6 +20,11 @@ func MoveCursorUp(_ uintptr) func(io.Writer, uintptr, int) error {
 	return PosixMoveCursorUp
 }
 
+// MoveCursorDown moves the cursor to the line n lines below the current one.
+func MoveCursorDown(_ uintptr) func(io.Writer, uintptr, int) error {
+	return PosixMoveCursorDown
+}
+
 // CanUpdateStatus returns true if status lines can be printed, the process
 // output is not redirected to a file or pipe.
 func CanUpdateStatus(fd uintptr) bool {

internal/terminal/terminal_windows.go

@@ -35,6 +35,17 @@ func MoveCursorUp(fd uintptr) func(io.Writer, uintptr, int) error {
 	return PosixMoveCursorUp
 }
 
+// moveCursorDown moves the cursor to the line n lines below the current one.
+func MoveCursorDown(fd uintptr) func(io.Writer, uintptr, int) error {
+	// easy case, the terminal is cmd or psh, without redirection
+	if isWindowsTerminal(fd) {
+		return windowsMoveCursorDown
+	}
+
+	// assume we're running in mintty/cygwin
+	return PosixMoveCursorDown
+}
+
 var kernel32 = syscall.NewLazyDLL("kernel32.dll")
 
 var (
@@ -73,6 +84,19 @@ func windowsMoveCursorUp(_ io.Writer, fd uintptr, n int) error {
 	return nil
 }
 
+// windowsMoveCursorDown moves the cursor to the line n lines below the current one.
+func windowsMoveCursorDown(_ io.Writer, fd uintptr, n int) error {
+	var info windows.ConsoleScreenBufferInfo
+	windows.GetConsoleScreenBufferInfo(windows.Handle(fd), &info)
+
+	// move cursor up by n lines and to the first column
+	windows.SetConsoleCursorPosition(windows.Handle(fd), windows.Coord{
+		X: 0,
+		Y: info.CursorPosition.Y + int16(n),
+	})
+	return nil
+}
+
 // isWindowsTerminal return true if the file descriptor is a windows terminal (cmd, psh).
 func isWindowsTerminal(fd uintptr) bool {
 	return term.IsTerminal(int(fd))

internal/ui/termstatus/status.go

@@ -26,7 +26,7 @@ type Terminal struct {
 	errWriter        io.Writer
 	msg              chan message
 	status           chan status
-	lastStatusLen    int
+	lastStatus       []string
 	inputIsTerminal  bool
 	outputIsTerminal bool
 	canUpdateStatus  bool
@@ -40,6 +40,7 @@ type Terminal struct {
 
 	clearCurrentLine func(io.Writer, uintptr) error
 	moveCursorUp     func(io.Writer, uintptr, int) error
+	moveCursorDown   func(io.Writer, uintptr, int) error
 }
 
 type message struct {
@@ -123,6 +124,7 @@ func New(rd io.ReadCloser, wr io.Writer, errWriter io.Writer, disableStatus bool
 			t.fd = d.Fd()
 			t.clearCurrentLine = terminal.ClearCurrentLine(t.fd)
 			t.moveCursorUp = terminal.MoveCursorUp(t.fd)
+			t.moveCursorDown = terminal.MoveCursorDown(t.fd)
 		}
 		if terminal.OutputIsTerminal(d.Fd()) {
 			t.outputIsTerminal = true
@@ -205,12 +207,11 @@ func (t *Terminal) Run(ctx context.Context) {
 // run listens on the channels and updates the terminal screen.
 func (t *Terminal) run(ctx context.Context) {
 	var status []string
-	var lastWrittenStatus []string
 	for {
 		select {
 		case <-ctx.Done():
 			if !terminal.IsProcessBackground(t.fd) {
-				t.writeStatus([]string{})
+				t.writeStatus([]string{}, false)
 			}
 
 			return
@@ -225,7 +226,7 @@ func (t *Terminal) run(ctx context.Context) {
 				continue
 			}
 			if err := t.clearCurrentLine(t.wr, t.fd); err != nil {
-				_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
+				t.logWriteErr(err)
 				continue
 			}
 
@@ -237,12 +238,11 @@ func (t *Terminal) run(ctx context.Context) {
 			}
 
 			if _, err := io.WriteString(dst, msg.line); err != nil {
-				_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
+				t.logWriteErr(err)
 				continue
 			}
 
-			t.writeStatus(status)
-			lastWrittenStatus = append([]string{}, status...)
+			t.writeStatus(status, false)
 		case stat := <-t.status:
 			status = append(status[:0], stat.lines...)
 
@@ -251,44 +251,72 @@ func (t *Terminal) run(ctx context.Context) {
 				continue
 			}
 
-			if !slices.Equal(status, lastWrittenStatus) {
-				t.writeStatus(status)
-				// Copy the status slice to avoid aliasing
-				lastWrittenStatus = append([]string{}, status...)
-			}
+			t.writeStatus(status, true)
 		}
 	}
 }
 
-func (t *Terminal) writeStatus(status []string) {
-	statusLen := len(status)
-	status = append([]string{}, status...)
-	for i := len(status); i < t.lastStatusLen; i++ {
-		// clear no longer used status lines
-		status = append(status, "")
-		if i > 0 {
-			// all lines except the last one must have a line break
-			status[i-1] = status[i-1] + "\n"
-		}
+func (t *Terminal) logWriteErr(err error) {
+	if err != nil {
+		_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
 	}
-	t.lastStatusLen = statusLen
+}
 
-	for _, line := range status {
-		if err := t.clearCurrentLine(t.wr, t.fd); err != nil {
-			_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
+func (t *Terminal) writeStatus(status []string, skipUnchanged bool) {
+	var unchanged []bool
+	if skipUnchanged {
+		if slices.Equal(status, t.lastStatus) {
+			return
 		}
+		unchanged = findUnchangedLines(status, t.lastStatus)
+	}
+
+	lastStatusLen := len(t.lastStatus)
+	// Copy the status slice to avoid aliasing
+	t.lastStatus = append([]string{}, status...)
+
+	// Extend to clear no longer used status lines
+	status = append([]string{}, status...)
+	for i := len(status); i < lastStatusLen; i++ {
+		status = append(status, "")
+	}
+
+	for i, line := range status {
+		if unchanged != nil && i < len(unchanged) && unchanged[i] {
+			// don't write unchanged lines every frame
+			if i < len(status)-1 {
+				// just move the cursor down to the next line
+				t.logWriteErr(t.moveCursorDown(t.wr, t.fd, 1))
+			}
+			continue
+		}
+
+		t.logWriteErr(t.clearCurrentLine(t.wr, t.fd))
 
 		_, err := t.wr.Write([]byte(line))
-		if err != nil {
-			_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
+		t.logWriteErr(err)
+		// all lines except the last one must be followed by a line break
+		if i < len(status)-1 {
+			_, err := t.wr.Write([]byte("\n"))
+			t.logWriteErr(err)
 		}
 	}
 
 	if len(status) > 0 {
-		if err := t.moveCursorUp(t.wr, t.fd, len(status)-1); err != nil {
-			_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
+		t.logWriteErr(t.moveCursorUp(t.wr, t.fd, len(status)-1))
+	}
+}
+
+func findUnchangedLines(curr, last []string) []bool {
+	unchanged := make([]bool, len(curr))
+
+	for i := range min(len(curr), len(last)) {
+		if curr[i] == last[i] {
+			unchanged[i] = true
 		}
 	}
+
+	return unchanged
 }
 
 // runWithoutStatus listens on the channels and just prints out the messages,
@@ -312,17 +340,15 @@ func (t *Terminal) runWithoutStatus(ctx context.Context) {
 				dst = t.wr
 			}
 
-			if _, err := io.WriteString(dst, msg.line); err != nil {
-				_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
-			}
+			_, err := io.WriteString(dst, msg.line)
+			t.logWriteErr(err)
 
 		case stat := <-t.status:
 			if !slices.Equal(stat.lines, lastStatus) {
 				for _, line := range stat.lines {
 					// Ensure that each message ends with exactly one newline.
-					if _, err := fmt.Fprintln(t.wr, strings.TrimRight(line, "\n")); err != nil {
-						_, _ = fmt.Fprintf(t.errWriter, "write failed: %v\n", err)
-					}
+					_, err := fmt.Fprintln(t.wr, strings.TrimRight(line, "\n"))
+					t.logWriteErr(err)
 				}
 				// Copy the status slice to avoid aliasing
 				lastStatus = append([]string{}, stat.lines...)
@@ -374,9 +400,6 @@ func sanitizeLines(lines []string, width int) []string {
 		if width > 0 {
 			line = ui.Truncate(line, width-2)
 		}
-		if i < len(lines)-1 { // Last line gets no line break.
-			line += "\n"
-		}
 		lines[i] = line
 	}
 	return lines

internal/ui/termstatus/status_test.go

@@ -20,31 +20,63 @@ func TestSetStatus(t *testing.T) {
 		cl   = terminal.PosixControlClearLine
 		home = terminal.PosixControlMoveCursorHome
 		up   = terminal.PosixControlMoveCursorUp
+
+		clearLn = home + cl
 	)
 
 	term.SetStatus([]string{"first"})
-	exp := home + cl + "first" + home
+	exp := clearLn + "first" + home
 
 	term.SetStatus([]string{""})
-	exp += home + cl + "" + home
+	exp += clearLn + "" + home
 
 	term.SetStatus([]string{})
-	exp += home + cl + "" + home
+	exp += clearLn + "" + home
 
 	// already empty status
 	term.SetStatus([]string{})
 
 	term.SetStatus([]string{"foo", "bar", "baz"})
-	exp += home + cl + "foo\n" + home + cl + "bar\n" +
-		home + cl + "baz" + home + up + up
+	exp += clearLn + "foo\n" + clearLn + "bar\n" + clearLn + "baz" + home + up + up
 
 	term.SetStatus([]string{"quux", "needs\nquote"})
-	exp += home + cl + "quux\n" +
-		home + cl + "\"needs\\nquote\"\n" +
-		home + cl + home + up + up // Clear third line
+	exp += clearLn + "quux\n" +
+		clearLn + "\"needs\\nquote\"\n" +
+		clearLn + home + up + up // Clear third line
 
 	cancel()
-	exp += home + cl + "\n" + home + cl + home + up // Status cleared
+	exp += clearLn + "\n" + clearLn + "" + home + up // Status cleared
+
+	<-term.closed
+	rtest.Equals(t, exp, buf.String())
+}
+
+func TestSetStatusUnchangedLines(t *testing.T) {
+	buf, term, cancel := setupStatusTest()
+
+	const (
+		cl   = terminal.PosixControlClearLine
+		home = terminal.PosixControlMoveCursorHome
+		up   = terminal.PosixControlMoveCursorUp
+		down = terminal.PosixControlMoveCursorDown
+
+		clearLn  = home + cl
+		stepDown = home + down
+	)
+
+	term.SetStatus([]string{"line1", "line2", "line3"})
+	exp := clearLn + "line1\n" + clearLn + "line2\n" + clearLn + "line3" + home + up + up
+
+	term.SetStatus([]string{"line1", "line2", "line3-changed"})
+	exp += stepDown + stepDown + clearLn + "line3-changed" + home + up + up
+
+	term.SetStatus([]string{"line1", "line2", "line3-changed"})
+
+	term.SetStatus([]string{"line1", "line2-new", "line3-changed"})
+	exp += stepDown + clearLn + "line2-new\n" + home + up + up
+
+	cancel()
+	exp += clearLn + "\n" + clearLn + "\n" + clearLn + "" + home + up + up
 
 	<-term.closed
 	rtest.Equals(t, exp, buf.String())
@@ -58,6 +90,7 @@ func setupStatusTest() (*bytes.Buffer, *Terminal, context.CancelFunc) {
 	term.fd = ^uintptr(0)
 	term.clearCurrentLine = terminal.PosixClearCurrentLine
 	term.moveCursorUp = terminal.PosixMoveCursorUp
+	term.moveCursorDown = terminal.PosixMoveCursorDown
 
 	ctx, cancel := context.WithCancel(context.Background())
 	go term.Run(ctx)
@@ -91,8 +124,8 @@ func TestSanitizeLines(t *testing.T) {
 	}{
 		{[]string{""}, 80, []string{""}},
 		{[]string{"too long test line"}, 10, []string{"too long"}},
-		{[]string{"too long test line", "text"}, 10, []string{"too long\n", "text"}},
-		{[]string{"too long test line", "second long test line"}, 10, []string{"too long\n", "second l"}},
+		{[]string{"too long test line", "text"}, 10, []string{"too long", "text"}},
+		{[]string{"too long test line", "second long test line"}, 10, []string{"too long", "second l"}},
 	}
 
 	for _, test := range tests {