diff --git a/changelog/unreleased/issue-5562 b/changelog/unreleased/issue-5562 new file mode 100644 index 000000000..446cdfa51 --- /dev/null +++ b/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 diff --git a/internal/terminal/terminal_posix.go b/internal/terminal/terminal_posix.go index 08527b777..1a3b0581d 100644 --- a/internal/terminal/terminal_posix.go +++ b/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 +} diff --git a/internal/terminal/terminal_unix.go b/internal/terminal/terminal_unix.go index 2065f50cf..1f2f35d1b 100644 --- a/internal/terminal/terminal_unix.go +++ b/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 { diff --git a/internal/terminal/terminal_windows.go b/internal/terminal/terminal_windows.go index 163173286..7d8713acf 100644 --- a/internal/terminal/terminal_windows.go +++ b/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)) diff --git a/internal/ui/termstatus/status.go b/internal/ui/termstatus/status.go index 94ea6adaa..46357eca3 100644 --- a/internal/ui/termstatus/status.go +++ b/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 diff --git a/internal/ui/termstatus/status_test.go b/internal/ui/termstatus/status_test.go index b19e00557..989d2e782 100644 --- a/internal/ui/termstatus/status_test.go +++ b/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 {