Erase 'p' on Terminal.app

Terminal.app fails to properly handle DECRQM and leaves 'p' on the
screen.

Fix #4893

src/tui/light.go

@@ -46,15 +46,21 @@ type termQuery struct {
 	reply *regexp.Regexp
 }
 
-// What we ask the terminal at startup, in the order the queries go out. A
-// terminal answers them in that order, so the cursor position query is last and
-// also ends the wait: every terminal fzf supports answers it, so once its reply
-// arrives, a query still unanswered is one the terminal does not know rather
-// than one we stopped waiting for too early.
-var startupQueries = []termQuery{
-	{"?2004$p", pasteModeRegexp},
-	{"6n", offsetRegexp},
-}
+var offsetQuery = termQuery{"6n", offsetRegexp}
+var pasteModeQuery = termQuery{"?2004$p", pasteModeRegexp}
+
+// What we ask the terminal at startup, in the order the queries go out.
+// A terminal answers them in that order, so the cursor position query is last
+// and also ends the wait: every terminal fzf supports answers it, so once its
+// reply arrives, a query still unanswered is one the terminal does not know
+// rather than one we stopped waiting for too early.
+//
+// Terminals that don't support the paste mode query (DECRQM) might leave
+// 'p' on the screen. To handle such cases, we query the position before and
+// after it, compare them, and clean the artifact if they don't match.
+//
+// Reference: https://ansicode.eversources.app/en/sequence/decrqm
+var startupQueries = []termQuery{offsetQuery, pasteModeQuery, offsetQuery}
 
 func (r *LightRenderer) Bell() {
 	r.flushRaw("\a")

src/tui/light_query_test.go

@@ -0,0 +1,128 @@
+//go:build !windows
+
+package tui
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+// Drives queryStartup against a terminal simulated by pipes, with the replies
+// already queued so the exchange is deterministic.
+func replyingTerminal(t *testing.T, replies string) (*LightRenderer, func() string) {
+	t.Helper()
+	inR, inW, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	outR, outW, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inW.WriteString(replies); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { inR.Close(); inW.Close(); outR.Close() })
+
+	r := &LightRenderer{ttyin: inR, ttyout: outW, escDelay: defaultEscDelay}
+	written := func() string {
+		outW.Close()
+		var sb strings.Builder
+		buf := make([]byte, 4096)
+		for {
+			n, err := outR.Read(buf)
+			sb.Write(buf[:n])
+			if err != nil {
+				break
+			}
+		}
+		return sb.String()
+	}
+	return r, written
+}
+
+const eraseEcho = "\b \b"
+
+func TestQueryStartup(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		replies  string
+		row, col int
+		paste    string // "", "true" or "false"
+		erase    int    // columns the echo took, and so erasures expected
+	}{
+		{
+			name:    "mode already set",
+			replies: "\x1b[5;10R\x1b[?2004;1$y\x1b[5;10R",
+			row:     4, col: 9, paste: "true",
+		},
+		{
+			name:    "mode reset",
+			replies: "\x1b[5;10R\x1b[?2004;2$y\x1b[5;10R",
+			row:     4, col: 9, paste: "false",
+		},
+		{
+			name:    "mode not recognized",
+			replies: "\x1b[5;10R\x1b[?2004;0$y\x1b[5;10R",
+			row:     4, col: 9,
+		},
+		{
+			// Answers the position queries but ignores DECRQM without printing
+			name:    "query ignored",
+			replies: "\x1b[5;10R\x1b[5;10R",
+			row:     4, col: 9,
+		},
+		{
+			// macOS Terminal.app: ends the sequence at '$' and prints the 'p'
+			name:    "query echoed",
+			replies: "\x1b[5;10R\x1b[5;11R",
+			row:     4, col: 9, erase: 1,
+		},
+		{
+			// A terminal that prints more of what it could not parse
+			name:    "longer echo",
+			replies: "\x1b[5;10R\x1b[5;13R",
+			row:     4, col: 9, erase: 3,
+		},
+		{
+			// The echo wrapped to the next line, where erasing would guess
+			name:    "echo wrapped",
+			replies: "\x1b[5;80R\x1b[6;1R",
+			row:     4, col: 79,
+		},
+		{
+			name:    "no reply at all",
+			replies: "",
+			row:     -1, col: -1,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r, written := replyingTerminal(t, tc.replies)
+			row, col, pasteWasSet := r.queryStartup()
+
+			if row != tc.row || col != tc.col {
+				t.Errorf("offset (%d,%d), want (%d,%d)", row, col, tc.row, tc.col)
+			}
+			paste := ""
+			if pasteWasSet != nil {
+				paste = "false"
+				if *pasteWasSet {
+					paste = "true"
+				}
+			}
+			if paste != tc.paste {
+				t.Errorf("pasteWasSet %q, want %q", paste, tc.paste)
+			}
+
+			out := written()
+			if got := strings.Count(out, eraseEcho); got != tc.erase {
+				t.Errorf("erased %d columns, want %d (wrote %q)", got, tc.erase, out)
+			}
+			// The paste mode query has to sit between two position queries
+			if want := "\x1b[6n\x1b[?2004$p\x1b[6n"; !strings.Contains(out, want) {
+				t.Errorf("queries %q, want %q in order", out, want)
+			}
+		})
+	}
+}

src/tui/light_unix.go

@@ -183,12 +183,19 @@ func (r *LightRenderer) queryTerminal(queries []termQuery) [][][]byte {
 
 func (r *LightRenderer) queryStartup() (row int, col int, pasteWasSet *bool) {
 	replies := r.queryTerminal(startupQueries)
-	if paste := replies[0]; paste != nil && paste[1][0] != '0' {
+	before, paste, after := replies[0], replies[1], replies[2]
+
+	if paste != nil && paste[1][0] != '0' {
 		// 1 = set, 3 = permanently set
 		set := paste[1][0] == '1' || paste[1][0] == '3'
 		pasteWasSet = &set
 	}
-	row, col = parseOffset(replies[1])
+	row, col = parseOffset(before)
+
+	// The cursor moved right likely because terminal doesn't support DECRQM
+	if row2, col2 := parseOffset(after); row >= 0 && row2 == row && col2 > col {
+		r.flushRaw(strings.Repeat("\b \b", col2-col))
+	}
 	return
 }
 
@@ -200,7 +207,7 @@ func parseOffset(reply [][]byte) (row int, col int) {
 }
 
 func (r *LightRenderer) findOffset() (row int, col int) {
-	return parseOffset(r.queryTerminal(startupQueries[1:])[0])
+	return parseOffset(r.queryTerminal([]termQuery{offsetQuery})[0])
 }
 
 func (r *LightRenderer) getch(cancellable bool, nonblock bool) (int, getCharResult) {