Note the lookback limit on unfinished string sequences

A payload longer than escapeLookback stops being seen as unfinished, so a
reply that is also split across reads still leaks. Recognizing it means
tracking the open sequence across reads rather than rescanning the tail,
which is more than this change should carry.

Reported by Copilot on #4926.

.github/ISSUE_TEMPLATE/issue_template.yml

@@ -3,10 +3,6 @@ name: Issue Template
 description: Report a problem or bug related to fzf to help us improve
 
 body:
-  - type: markdown
-    attributes:
-      value: ISSUES NOT FOLLOWING THIS TEMPLATE WILL BE CLOSED AND DELETED
-
   - type: checkboxes
     attributes:
       label: Checklist
@@ -32,7 +28,6 @@ body:
         - label: Linux
         - label: macOS
         - label: Windows
-        - label: Etc.
 
   - type: checkboxes
     attributes:
@@ -41,9 +36,23 @@ body:
         - label: bash
         - label: zsh
         - label: fish
+        - label: nushell
+        - label: PowerShell
 
   - type: textarea
     attributes:
       label: Problem / Steps to reproduce
     validations:
       required: true
+
+  - type: textarea
+    attributes:
+      label: How did you run into this?
+      description: |
+        What were you actually trying to do? Include the command line or
+        configuration from your real setup.
+
+        If you did not hit this in actual use, say how you found it
+        (reading the code, automated analysis, etc.).
+    validations:
+      required: true

CHANGELOG.md

@@ -1,6 +1,10 @@
 CHANGELOG
 =========
 
+0.74.5
+------
+- Fixed `--gap-line` cutting a grapheme cluster when filling the last cells of the line (#4920)
+
 0.74.4
 ------
 - Fixed an escape sequence split across reads being parsed as a fragment, which leaked the rest into the query (#4899)

src/tui/light.go

@@ -348,17 +348,90 @@ func getEnv(name string, defaultValue int) int {
 func csiContinues(b byte) bool { return b >= 0x20 && b <= 0x3f }
 func csiFinal(b byte) bool     { return b >= 0x40 && b <= 0x7e }
 
+// csiEnd returns the length of the CSI sequence at the start of the buffer, or
+// 0 if it has no final byte yet or is malformed.
+func csiEnd(buffer []byte) int {
+	for i := 2; i < len(buffer); i++ {
+		if csiFinal(buffer[i]) {
+			return i + 1
+		}
+		if !csiContinues(buffer[i]) {
+			return 0
+		}
+	}
+	return 0
+}
+
+// stringEnd returns the length of the string sequence (DCS, OSC or APC) at the
+// start of the buffer, or 0 if its terminator has not arrived.
+func stringEnd(buffer []byte) int {
+	if len(buffer) < 2 {
+		return 0
+	}
+	// Every one of them ends with ST. BEL ends an OSC as well, because xterm has
+	// always allowed it, but stopping at a BEL inside a DCS or APC payload would
+	// frame only its first half and leave the rest to be typed into the query.
+	bel := buffer[1] == ']'
+	for i := 2; i < len(buffer); i++ {
+		switch buffer[i] {
+		case '\a':
+			if bel {
+				return i + 1
+			}
+		case Esc.Byte():
+			if i+1 == len(buffer) {
+				return 0 // ST may still be arriving
+			}
+			if buffer[i+1] == '\\' {
+				return i + 2
+			}
+			// Any other ESC ends the string and introduces a sequence of its
+			// own, so frame only what precedes it and leave the ESC to be
+			// parsed again. Scanning past it would swallow that sequence too.
+			return i
+		}
+	}
+	return 0
+}
+
+// stringIntroducer reports whether the byte after ESC starts a string sequence.
+// Only the three that terminals actually reply with: OSC for colors, title and
+// clipboard, DCS for XTVERSION and XTGETTCAP, APC for Kitty graphics. SOS and PM
+// are left out, as nothing sends them and waiting for a terminator that will
+// never come would delay ALT-X and ALT-^.
+func stringIntroducer(b byte) bool {
+	switch b {
+	case 'P', ']', '_':
+		return true
+	}
+	return false
+}
+
 // incompleteEscape reports whether the buffer ends in an escape sequence that
 // has not been terminated yet. The read loop keeps waiting in that case, so the
 // parser is never handed a fragment to guess at.
 func incompleteEscape(buffer []byte) bool {
 	// Only the tail can hold a sequence still arriving. This runs once per byte
 	// read, so scanning all of a large paste would make the read quadratic.
+	//
+	// The limit is that a string sequence with a payload longer than this stops
+	// being seen as unfinished, so one that is also split across reads reaches
+	// the parser incomplete and its payload is typed into the query. Recognizing
+	// it would mean tracking the open sequence across reads instead of
+	// rescanning the tail.
 	tail := buffer
 	if len(tail) > escapeLookback {
 		tail = tail[len(tail)-escapeLookback:]
 	}
 	start := bytes.LastIndexByte(tail, Esc.Byte())
+	// A trailing ESC can be the first half of a string terminator. Reading it as
+	// a lone ESC ends the wait and hands the parser an unterminated sequence, so
+	// fall back to the one this ESC would have terminated.
+	if start == len(tail)-1 && start > 0 {
+		if prev := bytes.LastIndexByte(tail[:start], Esc.Byte()); prev >= 0 {
+			start = prev
+		}
+	}
 	if start < 0 || len(tail)-start < 2 {
 		return false
 	}
@@ -376,6 +449,9 @@ func incompleteEscape(buffer []byte) bool {
 	case 'O':
 		return len(tail)-start < 3
 	}
+	if stringIntroducer(tail[start+1]) {
+		return stringEnd(tail[start:]) == 0
+	}
 	return false
 }
 
@@ -483,8 +559,10 @@ func (r *LightRenderer) GetChar(cancellable bool) Event {
 		return Event{CtrlSlash, 0, nil}
 	case Esc.Byte():
 		ev := r.escSequence(&sz)
-		// Second chance
-		if ev.Type == Invalid {
+		// Second chance, but only for a sequence that has not finished
+		// arriving. Re-reading after a complete one blocks until the next
+		// keystroke, holding back whatever follows it in the buffer.
+		if ev.Type == Invalid && incompleteEscape(r.buffer) {
 			r.buffer, result, err = r.getBytes(true)
 			if err != nil {
 				return Event{Fatal, 0, nil}
@@ -525,7 +603,23 @@ func (r *LightRenderer) setCancel(f func()) {
 	r.mutex.Unlock()
 }
 
+// escSequence parses an escape sequence. A CSI sequence fzf has no event for is
+// dropped whole: consuming only the part that parsed leaves the rest to be read
+// as input and typed into the query.
 func (r *LightRenderer) escSequence(sz *int) Event {
+	ev := r.parseEscSequence(sz)
+	if ev.Type != Invalid || len(r.buffer) < 3 || r.buffer[1] != '[' {
+		return ev
+	}
+	// Only a framed sequence is dropped. One still missing its final byte may
+	// yet be arriving, and the caller gives it another chance.
+	if end := csiEnd(r.buffer); end > *sz {
+		*sz = end
+	}
+	return ev
+}
+
+func (r *LightRenderer) parseEscSequence(sz *int) Event {
 	if len(r.buffer) < 2 {
 		return Event{Esc, 0, nil}
 	}
@@ -987,6 +1081,22 @@ func (r *LightRenderer) escSequence(sz *int) Event {
 			} // r.buffer[2]
 		} // r.buffer[2]
 	} // r.buffer[1]
+	// Nothing matched. A framed sequence is dropped whole: reading its
+	// introducer as an ALT-key below would type the rest into the query.
+	// Unterminated ones are left alone, as that is how ALT-[ and ALT-] arrive.
+	if r.buffer[1] == '[' {
+		// A bare "\e[c" is ALT-[ followed by a character, not a CSI sequence
+		if end := csiEnd(r.buffer); end > 3 {
+			*sz = end
+			return Event{Invalid, 0, nil}
+		}
+	} else if stringIntroducer(r.buffer[1]) {
+		if end := stringEnd(r.buffer); end > 0 {
+			*sz = end
+			return Event{Invalid, 0, nil}
+		}
+	}
+
 	rest := bytes.NewBuffer(r.buffer[1:])
 	c, size, err := rest.ReadRune()
 	if err == nil {

src/tui/light_csi_test.go

@@ -0,0 +1,107 @@
+package tui
+
+import "testing"
+
+// An unrecognized CSI sequence must be consumed whole. Consuming only part of
+// it leaves the rest to be read as input and typed into the query.
+func TestUnknownCSISequence(t *testing.T) {
+	for _, c := range []struct {
+		sequence string
+		event    EventType
+		size     int
+	}{
+		// Key encodings fzf does not implement
+		{"\x1b[97;5u", Invalid, 7},
+		{"\x1b[127;5u", Invalid, 8},
+		{"\x1b[27;5;127~", Invalid, 11},
+		{"\x1b[57441;1u", Invalid, 10},
+		{"\x1b\x1b[97;5u", Invalid, 7}, // ALT prefixed, the first ESC is dropped
+
+		// Replies to queries fzf did not send, or sent and stopped waiting for
+		{"\x1b[?1;2c", Invalid, 7},
+		{"\x1b[>0;95;0c", Invalid, 10},
+
+		// Mouse report arriving while mouse input is off
+		{"\x1b[<0;1;1M", Invalid, 9},
+
+		// String sequences: OSC, DCS, APC, PM, SOS
+		{"\x1b]11;rgb:4a4a/4a4a/4a4a\x1b\\", Invalid, 25}, // background color reply
+		{"\x1b]0;a title\a", Invalid, 12},                 // BEL terminated
+		{"\x1bP>|kitty(0.48.2)\x1b\\", Invalid, 19},       // XTVERSION reply
+		{"\x1b_Gi=1;OK\x1b\\", Invalid, 11},
+		{"\x1bP\ax\x1b\\", Invalid, 6},      // BEL inside a DCS payload is not a terminator
+		{"\x1b]foo\x1bX\x1b\\", Invalid, 5}, // ESC aborts the string, consuming only "\e]foo"               // kitty graphics reply
+
+		// Left alone: this is how ALT-[ and ALT-] arrive
+		{"\x1b[a", Alt, 2},
+		{"\x1b]abc", Alt, 2},
+		{"\x1b]11;rgb:", Alt, 2}, // terminator has not arrived
+
+		// SOS and PM are not framed, so ALT-X and ALT-^ are not delayed
+		{"\x1bXsos\x1b\\", Alt, 2},
+		{"\x1b^status\x1b\\", Alt, 2},
+
+		// Left alone: no final byte yet, so the sequence may still be arriving
+		{"\x1b[", Invalid, 2},
+
+		// Recognized sequences keep their existing parsing
+		{"\x1b[1;5A", CtrlUp, 6},
+		{"\x1b[3;5~", CtrlDelete, 6},
+		{"\x1b[2~", Insert, 4},
+		{"\x1b[200~", BracketedPasteBegin, 6},
+		{"\x1b[Z", ShiftTab, 3},
+		{"\x1bOA", Up, 3},
+		{"\x1b[12;34R", Invalid, 8},
+		{"\x1b[?2004;2$y", Invalid, 11},
+	} {
+		r := &LightRenderer{buffer: []byte(c.sequence)}
+		sz := 1
+		event := r.escSequence(&sz)
+		if event.Type != c.event {
+			t.Errorf("escSequence(%q) = %s, want %s",
+				c.sequence, event.Type.String(), c.event.String())
+		}
+		if sz != c.size {
+			t.Errorf("escSequence(%q) consumed %d bytes, want %d", c.sequence, sz, c.size)
+		}
+	}
+}
+
+func TestStringEnd(t *testing.T) {
+	for _, c := range []struct {
+		buffer string
+		want   int
+	}{
+		{"\x1b]0;t\a", 6},
+		{"\x1bP\ax\x1b\\", 6}, // BEL in the payload ignored, ST ends it
+		{"\x1bP\a", 0},        // BEL does not terminate a DCS
+		{"\x1b_Gi=1\a", 0},    // nor an APC
+		{"\x1b]0;t\x1b\\", 7},
+		{"\x1b_G\x1b\\", 5},
+		{"\x1b]0;t", 0},     // no terminator
+		{"\x1b]0;t\x1b", 0}, // terminator half arrived
+	} {
+		if got := stringEnd([]byte(c.buffer)); got != c.want {
+			t.Errorf("stringEnd(%q) = %d, want %d", c.buffer, got, c.want)
+		}
+	}
+}
+
+func TestCsiEnd(t *testing.T) {
+	for _, c := range []struct {
+		buffer string
+		want   int
+	}{
+		{"\x1b[97;5u", 7},
+		{"\x1b[A", 3},
+		{"\x1b[<0;1;1M", 9},
+		{"\x1b[?2004;2$y", 11},
+		{"\x1b[97;5", 0},   // no final byte
+		{"\x1b[", 0},       // no final byte
+		{"\x1b[1\x01A", 0}, // malformed, do not frame it
+	} {
+		if got := csiEnd([]byte(c.buffer)); got != c.want {
+			t.Errorf("csiEnd(%q) = %d, want %d", c.buffer, got, c.want)
+		}
+	}
+}

src/tui/light_escape_test.go

@@ -51,3 +51,35 @@ func TestIncompleteEscape(t *testing.T) {
 		}
 	}
 }
+
+// String sequences must be waited for until their terminator arrives
+func TestIncompleteStringEscape(t *testing.T) {
+	for _, c := range []struct {
+		buffer string
+		want   bool
+	}{
+		{"\x1b]11;rgb:", true},
+		{"\x1bP>|kitty", true},
+		{"\x1b_Gi=1", true},
+		{"\x1b]0;title\a", false},
+		{"\x1b]0;title\x1b\\", false},
+		{"\x1bP>|kitty(0.48.2)\x1b\\", false},
+		{"ab\x1b]11;rgb:", true},
+
+		// A string terminator split across reads: the ESC has arrived, the
+		// backslash has not
+		{"\x1b]0;t\x1b", true},
+		{"\x1bP>|kitty\x1b", true},
+		{"\x1b_G\x1b", true},
+
+		// Complete sequence followed by a lone ESC, which is the ESC key
+		{"\x1b]0;title\a\x1b", false},
+		{"\x1b\x1b", false},
+		{"\x1ba\x1b", false},
+		{"\x1b[A\x1b", false},
+	} {
+		if got := incompleteEscape([]byte(c.buffer)); got != c.want {
+			t.Errorf("incompleteEscape(%q) = %v, want %v", c.buffer, got, c.want)
+		}
+	}
+}

src/util/util.go

@@ -105,16 +105,18 @@ func RepeatToFill(str string, length int, limit int) string {
 	rest := limit % length
 	output := strings.Repeat(str, times)
 	if rest > 0 {
-		for _, r := range str {
-			rest -= uniseg.StringWidth(string(r))
-			if rest < 0 {
-				break
-			}
-			output += string(r)
-			if rest == 0 {
+		// Iterate over grapheme clusters so that we don't cut a cluster in half
+		end := 0
+		graphemes := uniseg.NewGraphemes(str)
+		for rest > 0 && graphemes.Next() {
+			width := graphemes.Width()
+			if width > rest {
 				break
 			}
+			rest -= width
+			_, end = graphemes.Positions()
 		}
+		output += str[:end]
 	}
 	return output
 }

src/util/util_test.go

@@ -109,6 +109,27 @@ func TestRepeatToFill(t *testing.T) {
 	if RepeatToFill("abcde", 10, 42) != strings.Repeat("abcde", 4)+"abcde"[:2] {
 		t.Error("Expected:", strings.Repeat("abcde", 4)+"abcde"[:2])
 	}
+
+	// Should not cut a grapheme cluster in half
+	for _, test := range []struct {
+		str      string
+		limit    int
+		expected string
+	}{
+		{"a\u0301b", 1, "a\u0301"},
+		{"a\u0301b", 3, "a\u0301ba\u0301"},
+		{"a\u0301b", 4, "a\u0301ba\u0301b"},
+		{"a\u4e00", 2, "a"},
+		{"a\u4e00", 4, "a\u4e00a"},
+		{"-\U0001f468\u200d\U0001f469\u200d\U0001f467", 1, "-"},
+		{"-\U0001f468\u200d\U0001f469\u200d\U0001f467", 2, "-"},
+		{"-\U0001f468\u200d\U0001f469\u200d\U0001f467", 4, "-\U0001f468\u200d\U0001f469\u200d\U0001f467-"},
+	} {
+		actual := RepeatToFill(test.str, StringWidth(test.str), test.limit)
+		if actual != test.expected {
+			t.Errorf("Expected: %q, actual: %q", test.expected, actual)
+		}
+	}
 }
 
 func TestStringWidth(t *testing.T) {