diff --git a/CHANGELOG.md b/CHANGELOG.md index d8ac724..7bab86c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed +- `omnishell tui`: the filter and option-value fields move the cursor and + delete by grapheme cluster, so a combining accent, an emoji sequence or a flag + is handled as one character. + ## [0.7.0] - 2026-10-07 ### Added diff --git a/go.mod b/go.mod index 30ea29e..b09a657 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/BurntSushi/toml v1.6.0 github.com/charmbracelet/x/ansi v0.11.8 github.com/charmbracelet/x/term v0.2.2 + github.com/rivo/uniseg v0.4.7 github.com/spf13/cobra v1.10.2 ) @@ -23,7 +24,6 @@ require ( github.com/lucasb-eyer/go-colorful v1.4.1 // indirect github.com/mattn/go-runewidth v0.0.24 // indirect github.com/muesli/cancelreader v0.2.2 // indirect - github.com/rivo/uniseg v0.4.7 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect diff --git a/internal/tui/textedit.go b/internal/tui/textedit.go index 6b1218b..c72b7ff 100644 --- a/internal/tui/textedit.go +++ b/internal/tui/textedit.go @@ -3,35 +3,77 @@ package tui import ( tea "charm.land/bubbletea/v2" "github.com/charmbracelet/x/ansi" + "github.com/rivo/uniseg" ) // The two text fields (the filter and an option value) keep their text as a // string and the cursor as a rune index into it, so the Model stays a plain -// value and tests can read the text directly. +// value and tests can read the text directly. The cursor always rests on a +// grapheme-cluster boundary: a visible character made of several runes (a +// combining accent, a ZWJ emoji, a flag) is moved over and deleted as one. + +// clusterStarts returns the rune index at which each grapheme cluster of runes +// starts, followed by len(runes) as the final boundary. +func clusterStarts(runes []rune) []int { + starts := make([]int, 0, len(runes)+1) + rest, offset := string(runes), 0 + for rest != "" { + cluster, remainder, _, _ := uniseg.FirstGraphemeClusterInString(rest, -1) + starts = append(starts, offset) + offset += len([]rune(cluster)) + rest = remainder + } + return append(starts, len(runes)) +} + +// snapToBoundary returns the cluster boundary at or before pos, which it first +// clamps into the text. +func snapToBoundary(starts []int, pos int) int { + pos = min(max(pos, 0), starts[len(starts)-1]) + for i := len(starts) - 1; i >= 0; i-- { + if starts[i] <= pos { + return starts[i] + } + } + return 0 +} + +// boundaryIndex returns the index into starts of the boundary pos (already +// snapped). +func boundaryIndex(starts []int, pos int) int { + for i, s := range starts { + if s == pos { + return i + } + } + return len(starts) - 1 +} // editText applies one key press to text, with the cursor at pos, and returns // the new text and cursor. ok is false for keys it does not handle, so the // caller can give them their own meaning (enter, esc, ctrl+c). func editText(text string, pos int, msg tea.KeyPressMsg) (string, int, bool) { runes := []rune(text) - pos = min(max(pos, 0), len(runes)) + starts := clusterStarts(runes) + pos = snapToBoundary(starts, pos) + at := boundaryIndex(starts, pos) switch msg.String() { case "left", "ctrl+b": - pos = max(pos-1, 0) + pos = starts[max(at-1, 0)] case "right", "ctrl+f": - pos = min(pos+1, len(runes)) + pos = starts[min(at+1, len(starts)-1)] case "home", "ctrl+a": pos = 0 case "end", "ctrl+e": pos = len(runes) case "backspace": - if pos > 0 { - runes = append(runes[:pos-1], runes[pos:]...) - pos-- + if at > 0 { + runes = append(runes[:starts[at-1]], runes[pos:]...) + pos = starts[at-1] } case "delete", "ctrl+d": - if pos < len(runes) { - runes = append(runes[:pos], runes[pos+1:]...) + if at < len(starts)-1 { + runes = append(runes[:pos], runes[starts[at+1]:]...) } default: if msg.Text == "" { @@ -47,7 +89,7 @@ func editText(text string, pos int, msg tea.KeyPressMsg) (string, int, bool) { // the new text and the cursor behind the inserted part. func pasteText(text string, pos int, content string) (string, int) { runes := []rune(text) - pos = min(max(pos, 0), len(runes)) + pos = snapToBoundary(clusterStarts(runes), pos) add := []rune(sanitize(content)) out := make([]rune, 0, len(runes)+len(add)) out = append(out, runes[:pos]...) @@ -62,7 +104,7 @@ func pasteText(text string, pos int, content string) (string, int) { // right when much of it follows the cursor, so the cursor is always inside. func editWindow(text string, pos, room int) (string, int) { runes := []rune(text) - pos = min(max(pos, 0), len(runes)) + pos = snapToBoundary(clusterStarts(runes), pos) before, after := string(runes[:pos]), string(runes[pos:]) atEnd := after == "" diff --git a/internal/tui/textedit_test.go b/internal/tui/textedit_test.go index 4bee3da..5523feb 100644 --- a/internal/tui/textedit_test.go +++ b/internal/tui/textedit_test.go @@ -269,3 +269,92 @@ func TestNoCursorOutsideTextFields(t *testing.T) { t.Fatal("a kept filter has no cursor") } } + +// Clusters made of several runes, written as escapes so the source stays +// unambiguous: e + combining acute, a ZWJ family emoji, and a flag. +const ( + clusterAccent = "e\u0301" + clusterFamily = "\U0001F468\u200d\U0001F469\u200d\U0001F467" + clusterFlag = "\U0001F1E9\U0001F1EA" +) + +func TestEditTextMovesOverWholeClusters(t *testing.T) { + for name, cluster := range map[string]string{"accent": clusterAccent, "family": clusterFamily, "flag": clusterFlag} { + text := "a" + cluster + "z" + end := len([]rune(text)) + afterCluster := end - 1 + + _, pos, _ := editText(text, afterCluster, key("left")) + if pos != 1 { + t.Errorf("%s: left from behind the cluster gave pos %d, want 1", name, pos) + } + _, pos, _ = editText(text, 1, key("right")) + if pos != afterCluster { + t.Errorf("%s: right over the cluster gave pos %d, want %d", name, pos, afterCluster) + } + } +} + +func TestEditTextDeletesWholeClusters(t *testing.T) { + for name, cluster := range map[string]string{"accent": clusterAccent, "family": clusterFamily, "flag": clusterFlag} { + text := "a" + cluster + "z" + afterCluster := len([]rune(text)) - 1 + + got, pos, _ := editText(text, afterCluster, key("backspace")) + if got != "az" || pos != 1 { + t.Errorf("%s: backspace gave text=%q pos=%d, want az / 1", name, got, pos) + } + got, pos, _ = editText(text, 1, key("delete")) + if got != "az" || pos != 1 { + t.Errorf("%s: delete gave text=%q pos=%d, want az / 1", name, got, pos) + } + } +} + +func TestEditTextSnapsAMidClusterCursorToABoundary(t *testing.T) { + text := "a" + clusterFamily + "z" + _, pos, _ := editText(text, 3, key("right")) // 3 is inside the family emoji + if pos != len([]rune(text))-1 { + t.Fatalf("pos = %d, want the cursor behind the cluster", pos) + } + got, pos, _ := editText(text, 3, key("delete")) + if got != "az" || pos != 1 { + t.Fatalf("text=%q pos=%d, want the whole cluster removed", got, pos) + } +} + +func TestPasteTextSnapsAMidClusterCursorToABoundary(t *testing.T) { + got, pos := pasteText("a"+clusterAccent+"z", 2, "X") // between e and the accent + if got != "aXe\u0301z" || pos != 2 { + t.Fatalf("text=%q pos=%d, want X before the cluster, never inside it", got, pos) + } +} + +func TestEditWindowNeverCutsACluster(t *testing.T) { + for name, cluster := range map[string]string{"accent": clusterAccent, "family": clusterFamily, "flag": clusterFlag} { + text := strings.Repeat("ab"+cluster, 12) + runes := []rune(text) + for pos := 0; pos <= len(runes); pos++ { + cell, col := editWindow(text, pos, 9) + if w := ansi.StringWidth(cell); w > 9 || col < 0 || col >= 9 { + t.Fatalf("%s pos %d: cell %q is %d wide with cursor at %d", name, pos, cell, w, col) + } + for _, bad := range []string{"\u0301", "\u200d"} { + if strings.HasPrefix(strings.TrimPrefix(cell, "…"), bad) { + t.Fatalf("%s pos %d: cell %q starts inside a cluster", name, pos, cell) + } + } + if strings.Count(cell, "\U0001F1E9")+strings.Count(cell, "\U0001F1EA") == 1 { + t.Fatalf("%s pos %d: cell %q holds half a flag", name, pos, cell) + } + } + } +} + +func TestEditWindowPlacesTheCursorOnAClusterColumn(t *testing.T) { + text := "a" + clusterAccent + "z" + cell, col := editWindow(text, 2, 23) // mid-cluster input snaps to the boundary + if cell != text || col != 1 { + t.Fatalf("cell=%q col=%d, want the cursor on the accent cluster at column 1", cell, col) + } +}