From 9592f7c48cfa094c1fd79ddf3165fd5ef889c6a5 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 3 Nov 2026 21:38:03 +0100 Subject: [PATCH] Fix double-click text selection Assisted-By: Docker Agent --- pkg/tui/components/messages/clipboard.go | 132 ++---------------- pkg/tui/components/messages/clipboard_test.go | 28 ++++ pkg/tui/components/messages/lineutil.go | 22 ++- pkg/tui/components/messages/messages.go | 3 +- pkg/tui/components/messages/selection.go | 56 ++++---- pkg/tui/components/messages/selection_test.go | 125 +++++++++++++++++ 6 files changed, 217 insertions(+), 149 deletions(-) diff --git a/pkg/tui/components/messages/clipboard.go b/pkg/tui/components/messages/clipboard.go index 367aadad52..a7bb09d358 100644 --- a/pkg/tui/components/messages/clipboard.go +++ b/pkg/tui/components/messages/clipboard.go @@ -9,7 +9,6 @@ import ( tea "charm.land/bubbletea/v2" "github.com/atotto/clipboard" "github.com/charmbracelet/x/ansi" - "github.com/mattn/go-runewidth" "github.com/docker/docker-agent/pkg/tui/components/markdown" "github.com/docker/docker-agent/pkg/tui/components/notification" @@ -80,47 +79,6 @@ func stripBorderChars(s string) string { return result.String() } -// isWordChar returns true if the rune is a word character (letter, digit, or underscore) -func isWordChar(r rune) bool { - return (r >= 'a' && r <= 'z') || - (r >= 'A' && r <= 'Z') || - (r >= '0' && r <= '9') || - r == '_' || - r >= 0x80 // Include non-ASCII characters (unicode letters, etc.) -} - -// displayWidthToRuneIndex converts a display width to a rune index -func displayWidthToRuneIndex(s string, targetWidth int) int { - if targetWidth <= 0 { - return 0 - } - - runes := []rune(s) - currentWidth := 0 - - for i, r := range runes { - if currentWidth >= targetWidth { - return i - } - currentWidth += runewidth.RuneWidth(r) - } - - return len(runes) -} - -// runeIndexToDisplayWidth converts a rune index to display width -func runeIndexToDisplayWidth(s string, runeIdx int) int { - runes := []rune(s) - if runeIdx > len(runes) { - runeIdx = len(runes) - } - width := 0 - for i := range runeIdx { - width += runewidth.RuneWidth(runes[i]) - } - return width -} - // uiAffordanceLines lists the purely decorative click affordances (copy, // edit, retry labels, collapse indicator) that can appear on their own line // in rendered messages. Built as a slice because some labels share the same @@ -156,7 +114,7 @@ func isUIAffordanceLine(trimmed string) bool { // extractSelectedText extracts the currently selected text from rendered content func (m *model) extractSelectedText() string { - if !m.selection.active { + if !m.selection.hasRange() { return "" } @@ -172,53 +130,22 @@ func (m *model) extractSelectedText() string { var selected []string for i := startLine; i <= endLine && i < m.totalHeight; i++ { - originalLine := m.renderedLine(i) - // Strip ANSI codes first to get the displayed text with borders - plainLine := ansi.Strip(originalLine) - // Strip border characters to get the actual text content - line := stripBorderChars(plainLine) - runes := []rune(line) - - // Map visual column positions from the plain line (with borders) to the - // stripped line (without borders) by tracking which runes correspond to - // which visual columns - visualToRune := make(map[int]int) - visualCol := 0 - lineRuneIdx := 0 - for _, r := range plainLine { - if !boxDrawingChars[r] { - // This rune is kept in the stripped line - visualToRune[visualCol] = lineRuneIdx - lineRuneIdx++ - } - visualCol += runewidth.RuneWidth(r) + plainLine := ansi.Strip(m.renderedLine(i)) + from, to := 0, ansi.StringWidth(plainLine) + if i == startLine { + from = startCol + } + if i == endLine { + to = endCol } - // Find the closest rune index for the start and end columns - lineWidth := visualCol - startRuneIdx := findClosestRuneIndex(visualToRune, startCol, len(runes), lineWidth) - endRuneIdx := findClosestRuneIndex(visualToRune, endCol, len(runes), lineWidth) - - var lineText string - switch i { - case startLine: - if startLine == endLine { - if startRuneIdx < len(runes) && startRuneIdx < endRuneIdx { - lineText = string(runes[startRuneIdx:endRuneIdx]) - } - break + var text strings.Builder + for _, cluster := range lineGraphemes(plainLine) { + if cluster.endCol > from && cluster.startCol < to { + text.WriteString(cluster.text) } - // First line: from startCol to end - if startRuneIdx < len(runes) { - lineText = string(runes[startRuneIdx:]) - } - case endLine: - // Last line: from start to endCol - lineText = string(runes[:endRuneIdx]) - default: - // Middle lines: entire line - lineText = line } + lineText := stripBorderChars(text.String()) // Keep leading whitespace (indentation) but drop the width padding // that rendered lines carry on the right. @@ -273,39 +200,6 @@ func cleanSelectedLines(lines []string) string { return strings.Join(out, "\n") } -// findClosestRuneIndex finds the rune index for a given visual column, -// or the closest next rune if the exact column doesn't exist. -// Columns at or beyond the end of the line map to one past the last rune, so -// a selection dragged past the end of an unpadded line still includes its -// last character (slicing treats the end index as exclusive). -func findClosestRuneIndex(visualToRune map[int]int, visualCol, maxRunes, lineWidth int) int { - // Try exact match first - if runeIdx, ok := visualToRune[visualCol]; ok { - return runeIdx - } - - if visualCol >= lineWidth { - return maxRunes - } - - // Find the next available rune index after the visual column - for col := visualCol + 1; col <= visualCol+10; col++ { - if runeIdx, ok := visualToRune[col]; ok { - return runeIdx - } - } - - // Find the previous available rune index - for col := visualCol - 1; col >= 0; col-- { - if runeIdx, ok := visualToRune[col]; ok { - return runeIdx - } - } - - // Fallback: return the last rune index - return maxRunes -} - // copySelectionToClipboard copies the currently selected text to clipboard func (m *model) copySelectionToClipboard() tea.Cmd { if !m.selection.active { diff --git a/pkg/tui/components/messages/clipboard_test.go b/pkg/tui/components/messages/clipboard_test.go index dd80190963..734a3479f3 100644 --- a/pkg/tui/components/messages/clipboard_test.go +++ b/pkg/tui/components/messages/clipboard_test.go @@ -197,3 +197,31 @@ func TestCleanSelectedLines(t *testing.T) { }) } } + +func TestExtractSelectedTextGraphemeColumns(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + line string + start, end int + want string + }{ + {"empty wide selection", "界 next", 1, 1, ""}, + {"wide first cell", "界 next", 0, 1, "界"}, + {"wide second cell", "界 next", 1, 2, "界"}, + {"emoji", "👩‍💻 next", 0, 2, "👩‍💻"}, + {"after emoji", "👩‍💻 next", 3, 7, "next"}, + {"variation selector", "☀️ next", 0, 2, "☀️"}, + {"combining mark", "cafe\u0301 next", 0, 4, "cafe\u0301"}, + {"after combining mark", "cafe\u0301 next", 5, 9, "next"}, + {"border after emoji", "│👩‍💻│next│", 4, 8, "next"}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + m := newSelectionModel([]string{tt.line}) + m.selectRange(0, tt.start, 0, tt.end) + assert.Equal(t, tt.want, m.extractSelectedText()) + }) + } +} diff --git a/pkg/tui/components/messages/lineutil.go b/pkg/tui/components/messages/lineutil.go index d48ccf8990..f03e325d7c 100644 --- a/pkg/tui/components/messages/lineutil.go +++ b/pkg/tui/components/messages/lineutil.go @@ -3,15 +3,13 @@ package messages import ( "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" - "github.com/mattn/go-runewidth" ) // styleLineSegment applies a lipgloss style to the portion of a line between // startCol and endCol (display columns), preserving the text before and after. // ANSI codes in the styled segment are stripped so the style renders cleanly. func styleLineSegment(line string, startCol, endCol int, style lipgloss.Style) string { - plainLine := ansi.Strip(line) - plainWidth := runewidth.StringWidth(plainLine) + plainWidth := ansi.StringWidth(line) if startCol >= plainWidth || startCol >= endCol { return line @@ -24,3 +22,21 @@ func styleLineSegment(line string, startCol, endCol int, style lipgloss.Style) s return before + style.Render(segment) + after } + +// lineGrapheme keeps terminal columns aligned with whole grapheme clusters. +type lineGrapheme struct { + text string + startCol, endCol int +} + +func lineGraphemes(line string) []lineGrapheme { + var clusters []lineGrapheme + col := 0 + for line != "" { + cluster, width := ansi.FirstGraphemeCluster(line, ansi.GraphemeWidth) + clusters = append(clusters, lineGrapheme{text: cluster, startCol: col, endCol: col + width}) + line = line[len(cluster):] + col += width + } + return clusters +} diff --git a/pkg/tui/components/messages/messages.go b/pkg/tui/components/messages/messages.go index be7455cebb..ef7b73aa6b 100644 --- a/pkg/tui/components/messages/messages.go +++ b/pkg/tui/components/messages/messages.go @@ -616,6 +616,7 @@ func (m *model) handleMouseMotion(msg tea.MouseMotionMsg) (layout.Model, tea.Cmd // Any real movement turns a multi-click into a drag: the copy now // belongs to the release handler, not the debounced word copy. if line != m.selection.lastClickLine || col != m.selection.lastClickCol { + m.selection.dragged = true m.selection.pendingCopyID++ } m.selection.update(line, col) @@ -659,7 +660,7 @@ func (m *model) handleMouseRelease(msg tea.MouseReleaseMsg) (layout.Model, tea.C line, col := m.mouseToLineCol(msg.X, msg.Y) // If the mouse didn't move since the press, this wasn't a drag. - if line == m.selection.lastClickLine && col == m.selection.lastClickCol { + if !m.selection.dragged && line == m.selection.lastClickLine && col == m.selection.lastClickCol { if m.selection.hasRange() { // Word/line selection from a multi-click: keep it, the // debounced copy fires. diff --git a/pkg/tui/components/messages/selection.go b/pkg/tui/components/messages/selection.go index eb37f88b32..886049cd8f 100644 --- a/pkg/tui/components/messages/selection.go +++ b/pkg/tui/components/messages/selection.go @@ -3,10 +3,11 @@ package messages import ( "strings" "time" + "unicode" + "unicode/utf8" tea "charm.land/bubbletea/v2" "github.com/charmbracelet/x/ansi" - "github.com/mattn/go-runewidth" "github.com/docker/docker-agent/pkg/tui/styles" ) @@ -19,6 +20,7 @@ type selectionState struct { endLine int endCol int mouseButtonDown bool + dragged bool mouseY int // Screen Y coordinate for autoscroll // Word anchor for double-click drags: while anchored, the selection @@ -126,6 +128,7 @@ func (s *selectionState) normalized() (startLine, startCol, endLine, endCol int) // 2=double, 3=triple). Counting cycles after a triple-click so a fourth // quick click starts a fresh drag selection. func (s *selectionState) detectClickType(line, col int) int { + s.dragged = false now := time.Now() colDiff := col - s.lastClickCol isConsecutive := !s.lastClickTime.IsZero() && @@ -203,37 +206,34 @@ func (m *model) selectWordAt(line, col int) bool { return false } - originalLine := m.renderedLine(line) - plainLine := stripBorderChars(ansi.Strip(originalLine)) - if plainLine == "" { + // Keep table separators and borders in place so columns match the screen. + clusters := lineGraphemes(ansi.Strip(m.renderedLine(line))) + if len(clusters) == 0 { return false } - // Calculate border offset to adjust column position - borderOffset := runewidth.StringWidth(ansi.Strip(originalLine)) - runewidth.StringWidth(plainLine) - runes := []rune(plainLine) - - // Convert display column to rune index - runeIdx := min(max(0, displayWidthToRuneIndex(plainLine, max(0, col-borderOffset))), len(runes)-1) - if runeIdx < 0 { + idx := 0 + for idx < len(clusters)-1 && col >= clusters[idx].endCol { + idx++ + } + r, _ := utf8.DecodeRuneInString(clusters[idx].text) + if boxDrawingChars[r] { return false } + onWord, onSpace := isWordChar(r), unicode.IsSpace(r) + sameClass := func(cluster lineGrapheme) bool { + r, _ := utf8.DecodeRuneInString(cluster.text) + return !boxDrawingChars[r] && isWordChar(r) == onWord && unicode.IsSpace(r) == onSpace + } - // Find word boundaries - determine if we're on a word or non-word char - onWordChar := isWordChar(runes[runeIdx]) - startIdx, endIdx := runeIdx, runeIdx - - // Expand to find contiguous characters of the same type - for startIdx > 0 && isWordChar(runes[startIdx-1]) == onWordChar { + startIdx, endIdx := idx, idx + for startIdx > 0 && sameClass(clusters[startIdx-1]) { startIdx-- } - for endIdx < len(runes)-1 && isWordChar(runes[endIdx+1]) == onWordChar { + for endIdx < len(clusters)-1 && sameClass(clusters[endIdx+1]) { endIdx++ } - - // Convert rune indices back to display columns, accounting for border offset - startCol := runeIndexToDisplayWidth(plainLine, startIdx) + borderOffset - endCol := runeIndexToDisplayWidth(plainLine, endIdx+1) + borderOffset + startCol, endCol := clusters[startIdx].startCol, clusters[endIdx].endCol // Set selection m.selection.active = true @@ -246,6 +246,10 @@ func (m *model) selectWordAt(line, col int) bool { return true } +func isWordChar(r rune) bool { + return unicode.IsLetter(r) || unicode.IsNumber(r) || unicode.IsMark(r) || r == '_' +} + // selectLineAt selects the entire line at the given line position. // It reports whether a non-blank line was actually selected. func (m *model) selectLineAt(line int) bool { @@ -262,9 +266,9 @@ func (m *model) selectLineAt(line int) bool { } // Find start column: position of first non-whitespace character - startCol := runewidth.StringWidth(plainLine) - runewidth.StringWidth(strings.TrimLeft(plainLine, " \t")) + startCol := ansi.StringWidth(plainLine) - ansi.StringWidth(strings.TrimLeft(plainLine, " \t")) // Find end column: position after last non-whitespace character - endCol := runewidth.StringWidth(strings.TrimRight(plainLine, " \t")) + endCol := ansi.StringWidth(strings.TrimRight(plainLine, " \t")) // Set selection to cover only the text content (excluding padding/borders) m.selection.active = true @@ -285,8 +289,8 @@ func (m *model) applySelectionHighlight(lines []string, viewportStartLine int) [ getLineBounds := func(line string) (textStart, textEnd int) { plainLine := ansi.Strip(line) - textStart = runewidth.StringWidth(plainLine) - runewidth.StringWidth(strings.TrimLeft(plainLine, " \t")) - textEnd = runewidth.StringWidth(strings.TrimRight(plainLine, " \t")) + textStart = ansi.StringWidth(plainLine) - ansi.StringWidth(strings.TrimLeft(plainLine, " \t")) + textEnd = ansi.StringWidth(strings.TrimRight(plainLine, " \t")) return textStart, textEnd } diff --git a/pkg/tui/components/messages/selection_test.go b/pkg/tui/components/messages/selection_test.go index 4432a9beb6..09b8cab609 100644 --- a/pkg/tui/components/messages/selection_test.go +++ b/pkg/tui/components/messages/selection_test.go @@ -295,3 +295,128 @@ func TestClickOnCopyLabelFlashesCopied(t *testing.T) { out = ansi.Strip(m.View()) assert.Contains(t, out, types.MessageCopyLabel) } + +func TestDoubleClickRenderedMarkdownWords(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + markdown string + word string + }{ + {"table first cell", "| First | Second | Third |\n| --- | --- | --- |\n| alpha | bravo | charlie |", "alpha"}, + {"table middle cell", "| First | Second | Third |\n| --- | --- | --- |\n| alpha | bravo | charlie |", "bravo"}, + {"table last cell", "| First | Second | Third |\n| --- | --- | --- |\n| alpha | bravo | charlie |", "charlie"}, + {"bold", "A **bold** word.", "bold"}, + {"inline code", "Run `command_name` now.", "command_name"}, + {"blockquote", "> quoted text", "quoted"}, + {"unicode quotes", "Say “hello” now.", "hello"}, + {"unicode dash", "hello—world", "world"}, + {"nonbreaking space", "hello\u00a0world", "hello"}, + {"wide character", "界 next", "界"}, + {"combining mark", "cafe\u0301 next", "cafe\u0301"}, + {"emoji prefix", "👩‍💻hello next", "hello"}, + {"emoji", "👩‍💻 next", "👩‍💻"}, + {"emoji variation selector", "☀️ next", "☀️"}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rendered, err := markdown.NewRenderer(80).Render(tt.markdown) + require.NoError(t, err) + lines := strings.Split(rendered, "\n") + for line, renderedLine := range lines { + before, _, ok := strings.Cut(ansi.Strip(renderedLine), tt.word) + if !ok { + continue + } + start := ansi.StringWidth(before) + for col := start; col < start+ansi.StringWidth(tt.word); col++ { + m := newSelectionModel(lines) + for range 2 { + m.handleMouseClick(tea.MouseClickMsg{X: col, Y: line, Button: tea.MouseLeft}) + m.handleMouseRelease(tea.MouseReleaseMsg{X: col, Y: line, Button: tea.MouseLeft}) + } + require.True(t, m.selection.anchored, "column %d", col) + assert.Equal(t, tt.word, m.extractSelectedText(), "column %d", col) + assert.Equal(t, start, m.selection.startCol, "column %d", col) + assert.Equal(t, start+ansi.StringWidth(tt.word), m.selection.endCol, "column %d", col) + } + return + } + t.Fatalf("word %q not found in rendered markdown %q", tt.word, ansi.Strip(rendered)) + }) + } +} + +func TestSelectWordAtKeepsBorderCoordinates(t *testing.T) { + t.Parallel() + + for _, line := range []string{"│alpha│bravo│charlie│", "┃ alpha ┃", "alpha │ bravo │ charlie"} { + for _, word := range []string{"alpha", "bravo", "charlie"} { + before, _, ok := strings.Cut(line, word) + if !ok { + continue + } + start := ansi.StringWidth(before) + for col := start; col < start+len(word); col++ { + m := newSelectionModel([]string{line}) + require.True(t, m.selectWordAt(0, col)) + assert.Equal(t, word, m.extractSelectedText(), "line %q column %d", line, col) + assert.Equal(t, start, m.selection.startCol) + assert.Equal(t, start+len(word), m.selection.endCol) + } + } + } +} + +func TestMultiClickDragBackToPressCopiesSelection(t *testing.T) { + t.Parallel() + + for _, clicks := range []int{2, 3} { + m := newSelectionModel([]string{"hello world"}) + for range clicks - 1 { + m.handleMouseClick(tea.MouseClickMsg{X: 2, Y: 0, Button: tea.MouseLeft}) + m.handleMouseRelease(tea.MouseReleaseMsg{X: 2, Y: 0, Button: tea.MouseLeft}) + } + m.handleMouseClick(tea.MouseClickMsg{X: 2, Y: 0, Button: tea.MouseLeft}) + pendingID := m.selection.pendingCopyID + m.handleMouseMotion(tea.MouseMotionMsg{X: 3, Y: 0}) + m.handleMouseMotion(tea.MouseMotionMsg{X: 2, Y: 0}) + _, cmd := m.handleMouseRelease(tea.MouseReleaseMsg{X: 2, Y: 0, Button: tea.MouseLeft}) + require.NotNil(t, cmd, "click count %d", clicks) + assert.False(t, m.IsSelecting()) + assert.Nil(t, m.handleDebouncedCopy(DebouncedCopyMsg{ClickID: pendingID})) + if clicks == 2 { + assert.Equal(t, "hello", m.extractSelectedText()) + } else { + assert.Equal(t, "hello world", m.extractSelectedText()) + } + } +} + +func TestSelectionHighlightPreservesGraphemes(t *testing.T) { + t.Parallel() + + for _, line := range []string{"界 next", "👩‍💻hello next", "☀️ next", "cafe\u0301 next"} { + m := newSelectionModel([]string{line}) + require.True(t, m.selectWordAt(0, 1)) + highlighted := m.highlightLine(line, m.selection.startCol, m.selection.endCol) + assert.Equal(t, line, ansi.Strip(highlighted)) + assert.Equal(t, ansi.StringWidth(line), ansi.StringWidth(highlighted)) + } +} + +func TestSingleClickDragBackToPressDoesNotCopy(t *testing.T) { + t.Parallel() + + for _, line := range []string{"hello world", "界 next", "👩‍💻 next"} { + m := newSelectionModel([]string{line}) + m.handleMouseClick(tea.MouseClickMsg{X: 1, Y: 0, Button: tea.MouseLeft}) + m.handleMouseMotion(tea.MouseMotionMsg{X: 3, Y: 0}) + m.handleMouseMotion(tea.MouseMotionMsg{X: 1, Y: 0}) + _, cmd := m.handleMouseRelease(tea.MouseReleaseMsg{X: 1, Y: 0, Button: tea.MouseLeft}) + assert.Nil(t, cmd, "line %q", line) + assert.Empty(t, m.extractSelectedText(), "line %q", line) + assert.False(t, m.IsSelecting()) + } +}