fix(copy): preserve range validation and reuse rendered content bounds
- commit
- 1324577f11725cb0e64bf3743b72c13a7ee16304
- author
- brice · · 2026-10-05 07:25 UTC
- parent
- 3f3a435
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +33 -14
M internal/usecase/copy/content_render_test.go +1 -1
| @@ -61,7 +61,7 @@ func TestRenderRowsSelectionHighlightsContentOnly(t *testing.T) { | |||
| 61 | 61 | copied.WriteByte('\n') | |
| 62 | 62 | } | |
| 63 | 63 | } | |
| 64 | - | require.Equal(t, m.SelectedText(), copied.String(), "highlighted glyphs match copied text") | |
| 64 | + | require.Equal(t, copied.String(), m.SelectedText(), "highlighted glyphs match copied text") | |
| 65 | 65 | for y, source := range tt.rows { | |
| 66 | 66 | require.Equal(t, source, doc.Row(y)) | |
| 67 | 67 | } | |
| @@ -61,7 +61,7 @@ func TestRenderRowsSelectionHighlightsContentOnly(t *testing.T) { | |||||
| 61 | copied.WriteByte('\n') | 61 | copied.WriteByte('\n') | ||
| 62 | } | 62 | } | ||
| 63 | } | 63 | } | ||
| 64 | - | require.Equal(t, m.SelectedText(), copied.String(), "highlighted glyphs match copied text") | 64 | + | require.Equal(t, copied.String(), m.SelectedText(), "highlighted glyphs match copied text") |
| 65 | for y, source := range tt.rows { | 65 | for y, source := range tt.rows { | ||
| 66 | require.Equal(t, source, doc.Row(y)) | 66 | require.Equal(t, source, doc.Row(y)) | ||
| 67 | } | 67 | } | ||
M internal/usecase/copy/content_selection_test.go +3 -0
| @@ -45,6 +45,9 @@ func TestDocumentExtractSelectedEmptyRanges(t *testing.T) { | |||
| 45 | 45 | want string | |
| 46 | 46 | }{ | |
| 47 | 47 | {"empty", []CellRange{{Row: 0, End: -1}}, ""}, | |
| 48 | + | {"reversed", []CellRange{{Row: 2, Start: 7, End: 0}}, "cd"}, | |
| 49 | + | {"past end", []CellRange{{Row: 0, Start: 99, End: 50}, {Row: 2, End: 1}}, "cd"}, | |
| 50 | + | {"negative", []CellRange{{Row: 0, Start: -3, End: -9}, {Row: 2, End: 1}}, "cd"}, | |
| 48 | 51 | {"blank middle", []CellRange{{Row: 0, End: 1}, {Row: 1, End: -1}, {Row: 2, End: 1}}, "ab\n\ncd"}, | |
| 49 | 52 | } | |
| 50 | 53 | for _, tt := range tests { | |
| @@ -45,6 +45,9 @@ func TestDocumentExtractSelectedEmptyRanges(t *testing.T) { | |||||
| 45 | want string | 45 | want string | ||
| 46 | }{ | 46 | }{ | ||
| 47 | {"empty", []CellRange{{Row: 0, End: -1}}, ""}, | 47 | {"empty", []CellRange{{Row: 0, End: -1}}, ""}, | ||
| 48 | + | {"reversed", []CellRange{{Row: 2, Start: 7, End: 0}}, "cd"}, | |||
| 49 | + | {"past end", []CellRange{{Row: 0, Start: 99, End: 50}, {Row: 2, End: 1}}, "cd"}, | |||
| 50 | + | {"negative", []CellRange{{Row: 0, Start: -3, End: -9}, {Row: 2, End: 1}}, "cd"}, | |||
| 48 | {"blank middle", []CellRange{{Row: 0, End: 1}, {Row: 1, End: -1}, {Row: 2, End: 1}}, "ab\n\ncd"}, | 51 | {"blank middle", []CellRange{{Row: 0, End: 1}, {Row: 1, End: -1}, {Row: 2, End: 1}}, "ab\n\ncd"}, | ||
| 49 | } | 52 | } | ||
| 50 | for _, tt := range tests { | 53 | for _, tt := range tests { | ||
M internal/usecase/copy/document.go +9 -5
| @@ -16,6 +16,7 @@ type Pos struct { | |||
| 16 | 16 | } | |
| 17 | 17 | ||
| 18 | 18 | // CellRange is an inclusive display-cell range on one physical document row. | |
| 19 | + | // End == Start-1 with an in-bounds Start denotes a selected row with no cells. | |
| 19 | 20 | type CellRange struct { | |
| 20 | 21 | Row int | |
| 21 | 22 | Start int | |
| @@ -285,7 +286,7 @@ func (d *Document) rangeText(r CellRange, logicalEnd bool) (string, bool, bool) | |||
| 285 | 286 | if len(row) == 0 { | |
| 286 | 287 | return "", false, r.Start == 0 && r.End == 0 | |
| 287 | 288 | } | |
| 288 | - | if r.End < r.Start { | |
| 289 | + | if r.End == r.Start-1 && r.Start >= 0 && r.Start <= len(row) { | |
| 289 | 290 | bound := d.snapshot.Bound(r.Row) | |
| 290 | 291 | return "", bound.Soft && !logicalEnd, true | |
| 291 | 292 | } | |
| @@ -303,9 +304,10 @@ func (d *Document) rangeText(r CellRange, logicalEnd bool) (string, bool, bool) | |||
| 303 | 304 | start = head.Col | |
| 304 | 305 | ||
| 305 | 306 | bound := d.snapshot.Bound(r.Row) | |
| 306 | - | soft := bound.Soft && !logicalEnd && end >= d.contentEnd(r.Row)-1 | |
| 307 | + | limit := contentEndOf(row, bound) | |
| 308 | + | soft := bound.Soft && !logicalEnd && end >= limit-1 | |
| 307 | 309 | if soft { | |
| 308 | - | end = min(end, d.contentEnd(r.Row)-1) | |
| 310 | + | end = min(end, limit-1) | |
| 309 | 311 | } else { | |
| 310 | 312 | end = lastContentCol(row, start, end) | |
| 311 | 313 | } | |
| @@ -323,11 +325,13 @@ func (d *Document) rangeText(r CellRange, logicalEnd bool) (string, bool, bool) | |||
| 323 | 325 | // contentEnd returns the exclusive display-cell end of row's copyable content. | |
| 324 | 326 | // Soft rows end at their logical bound; hard rows drop blank padding. | |
| 325 | 327 | func (d *Document) contentEnd(row int) int { | |
| 326 | - | cells := d.Row(row) | |
| 328 | + | return contentEndOf(d.Row(row), d.snapshot.Bound(row)) | |
| 329 | + | } | |
| 330 | + | ||
| 331 | + | func contentEndOf(cells []renderer.Cell, bound vt.LineBound) int { | |
| 327 | 332 | if len(cells) == 0 { | |
| 328 | 333 | return 0 | |
| 329 | 334 | } | |
| 330 | - | bound := d.snapshot.Bound(row) | |
| 331 | 335 | if bound.Soft { | |
| 332 | 336 | return min(max(bound.End, 0), len(cells)) | |
| 333 | 337 | } | |
| @@ -16,6 +16,7 @@ type Pos struct { | |||||
| 16 | } | 16 | } | ||
| 17 | 17 | ||||
| 18 | // CellRange is an inclusive display-cell range on one physical document row. | 18 | // CellRange is an inclusive display-cell range on one physical document row. | ||
| 19 | + | // End == Start-1 with an in-bounds Start denotes a selected row with no cells. | |||
| 19 | type CellRange struct { | 20 | type CellRange struct { | ||
| 20 | Row int | 21 | Row int | ||
| 21 | Start int | 22 | Start int | ||
| @@ -285,7 +286,7 @@ func (d *Document) rangeText(r CellRange, logicalEnd bool) (string, bool, bool) | |||||
| 285 | if len(row) == 0 { | 286 | if len(row) == 0 { | ||
| 286 | return "", false, r.Start == 0 && r.End == 0 | 287 | return "", false, r.Start == 0 && r.End == 0 | ||
| 287 | } | 288 | } | ||
| 288 | - | if r.End < r.Start { | 289 | + | if r.End == r.Start-1 && r.Start >= 0 && r.Start <= len(row) { |
| 289 | bound := d.snapshot.Bound(r.Row) | 290 | bound := d.snapshot.Bound(r.Row) | ||
| 290 | return "", bound.Soft && !logicalEnd, true | 291 | return "", bound.Soft && !logicalEnd, true | ||
| 291 | } | 292 | } | ||
| @@ -303,9 +304,10 @@ func (d *Document) rangeText(r CellRange, logicalEnd bool) (string, bool, bool) | |||||
| 303 | start = head.Col | 304 | start = head.Col | ||
| 304 | 305 | ||||
| 305 | bound := d.snapshot.Bound(r.Row) | 306 | bound := d.snapshot.Bound(r.Row) | ||
| 306 | - | soft := bound.Soft && !logicalEnd && end >= d.contentEnd(r.Row)-1 | 307 | + | limit := contentEndOf(row, bound) |
| 308 | + | soft := bound.Soft && !logicalEnd && end >= limit-1 | |||
| 307 | if soft { | 309 | if soft { | ||
| 308 | - | end = min(end, d.contentEnd(r.Row)-1) | 310 | + | end = min(end, limit-1) |
| 309 | } else { | 311 | } else { | ||
| 310 | end = lastContentCol(row, start, end) | 312 | end = lastContentCol(row, start, end) | ||
| 311 | } | 313 | } | ||
| @@ -323,11 +325,13 @@ func (d *Document) rangeText(r CellRange, logicalEnd bool) (string, bool, bool) | |||||
| 323 | // contentEnd returns the exclusive display-cell end of row's copyable content. | 325 | // contentEnd returns the exclusive display-cell end of row's copyable content. | ||
| 324 | // Soft rows end at their logical bound; hard rows drop blank padding. | 326 | // Soft rows end at their logical bound; hard rows drop blank padding. | ||
| 325 | func (d *Document) contentEnd(row int) int { | 327 | func (d *Document) contentEnd(row int) int { | ||
| 326 | - | cells := d.Row(row) | 328 | + | return contentEndOf(d.Row(row), d.snapshot.Bound(row)) |
| 329 | + | } | |||
| 330 | + | ||||
| 331 | + | func contentEndOf(cells []renderer.Cell, bound vt.LineBound) int { | |||
| 327 | if len(cells) == 0 { | 332 | if len(cells) == 0 { | ||
| 328 | return 0 | 333 | return 0 | ||
| 329 | } | 334 | } | ||
| 330 | - | bound := d.snapshot.Bound(row) | |||
| 331 | if bound.Soft { | 335 | if bound.Soft { | ||
| 332 | return min(max(bound.End, 0), len(cells)) | 336 | return min(max(bound.End, 0), len(cells)) | ||
| 333 | } | 337 | } | ||
M internal/usecase/copy/mode.go +4 -2
| @@ -604,11 +604,13 @@ func (m *Mode) RenderRowsRange(start, end int, paint func(int, []renderer.Cell), | |||
| 604 | 604 | } | |
| 605 | 605 | cursorCovered := false | |
| 606 | 606 | if hasSelectionBounds { | |
| 607 | - | if r, ok := selectionBounds.rangeForRow(d, src); ok { | |
| 607 | + | bound := d.snapshot.Bound(src) | |
| 608 | + | limit := contentEndOf(row, bound) | |
| 609 | + | if r, ok := selectionBounds.rangeForCells(src, row, limit); ok { | |
| 608 | 610 | for x := max(r.Start, 0); x <= min(r.End, len(row)-1); x++ { | |
| 609 | 611 | toggleInverse(&row[x].Style) | |
| 610 | 612 | } | |
| 611 | - | if col, ok := selectionBounds.newlineMarker(d, src); ok && col < len(row) { | |
| 613 | + | if col, ok := selectionBounds.newlineMarkerAt(src, limit, d.Width(), bound.Soft); ok && col < len(row) { | |
| 612 | 614 | toggleInverse(&row[col].Style) | |
| 613 | 615 | if cursorValid && cursor.Row == src && cursor.Col == col { | |
| 614 | 616 | cursorCovered = true | |
| @@ -604,11 +604,13 @@ func (m *Mode) RenderRowsRange(start, end int, paint func(int, []renderer.Cell), | |||||
| 604 | } | 604 | } | ||
| 605 | cursorCovered := false | 605 | cursorCovered := false | ||
| 606 | if hasSelectionBounds { | 606 | if hasSelectionBounds { | ||
| 607 | - | if r, ok := selectionBounds.rangeForRow(d, src); ok { | 607 | + | bound := d.snapshot.Bound(src) |
| 608 | + | limit := contentEndOf(row, bound) | |||
| 609 | + | if r, ok := selectionBounds.rangeForCells(src, row, limit); ok { | |||
| 608 | for x := max(r.Start, 0); x <= min(r.End, len(row)-1); x++ { | 610 | for x := max(r.Start, 0); x <= min(r.End, len(row)-1); x++ { | ||
| 609 | toggleInverse(&row[x].Style) | 611 | toggleInverse(&row[x].Style) | ||
| 610 | } | 612 | } | ||
| 611 | - | if col, ok := selectionBounds.newlineMarker(d, src); ok && col < len(row) { | 613 | + | if col, ok := selectionBounds.newlineMarkerAt(src, limit, d.Width(), bound.Soft); ok && col < len(row) { |
| 612 | toggleInverse(&row[col].Style) | 614 | toggleInverse(&row[col].Style) | ||
| 613 | if cursorValid && cursor.Row == src && cursor.Col == col { | 615 | if cursorValid && cursor.Row == src && cursor.Col == col { | ||
| 614 | cursorCovered = true | 616 | cursorCovered = true | ||
M internal/usecase/copy/selection.go +16 -6
| @@ -63,8 +63,9 @@ func (s Selection) Ranges(doc *Document) []CellRange { | |||
| 63 | 63 | return ranges | |
| 64 | 64 | } | |
| 65 | 65 | ||
| 66 | - | // RangeForRow returns the canonical inclusive range for one document row. It | |
| 67 | - | // lets viewport renderers avoid materializing ranges for off-screen rows. | |
| 66 | + | // RangeForRow returns the canonical inclusive range for one document row. | |
| 67 | + | // End == Start-1 denotes a selected row with no cells. It lets viewport | |
| 68 | + | // renderers avoid materializing ranges for off-screen rows. | |
| 68 | 69 | func (s Selection) RangeForRow(doc *Document, row int) (CellRange, bool) { | |
| 69 | 70 | bounds, ok := s.bounds(doc) | |
| 70 | 71 | if !ok { | |
| @@ -120,10 +121,16 @@ func (b selectionBounds) rangeForRow(doc *Document, row int) (CellRange, bool) { | |||
| 120 | 121 | return CellRange{}, false | |
| 121 | 122 | } | |
| 122 | 123 | cells := doc.Row(row) | |
| 124 | + | return b.rangeForCells(row, cells, contentEndOf(cells, doc.snapshot.Bound(row))) | |
| 125 | + | } | |
| 126 | + | ||
| 127 | + | func (b selectionBounds) rangeForCells(row int, cells []renderer.Cell, limit int) (CellRange, bool) { | |
| 128 | + | if row < b.start.Row || row > b.end.Row { | |
| 129 | + | return CellRange{}, false | |
| 130 | + | } | |
| 123 | 131 | if len(cells) == 0 { | |
| 124 | 132 | return CellRange{Row: row}, true | |
| 125 | 133 | } | |
| 126 | - | limit := doc.contentEnd(row) | |
| 127 | 134 | if limit == 0 { | |
| 128 | 135 | return CellRange{Row: row, End: -1}, true | |
| 129 | 136 | } | |
| @@ -147,11 +154,14 @@ func (b selectionBounds) rangeForRow(doc *Document, row int) (CellRange, bool) { | |||
| 147 | 154 | ||
| 148 | 155 | // newlineMarker reports the column marking a selected hard newline on row. | |
| 149 | 156 | func (b selectionBounds) newlineMarker(doc *Document, row int) (int, bool) { | |
| 150 | - | if row < b.start.Row || row > b.end.Row || doc.snapshot.Bound(row).Soft || (!b.linewise && row == b.end.Row) { | |
| 157 | + | return b.newlineMarkerAt(row, doc.contentEnd(row), doc.Width(), doc.snapshot.Bound(row).Soft) | |
| 158 | + | } | |
| 159 | + | ||
| 160 | + | func (b selectionBounds) newlineMarkerAt(row, end, width int, soft bool) (int, bool) { | |
| 161 | + | if row < b.start.Row || row > b.end.Row || soft || (!b.linewise && row == b.end.Row) { | |
| 151 | 162 | return 0, false | |
| 152 | 163 | } | |
| 153 | - | end := doc.contentEnd(row) | |
| 154 | - | return end, end > 0 && end < doc.Width() | |
| 164 | + | return end, end > 0 && end < width | |
| 155 | 165 | } | |
| 156 | 166 | ||
| 157 | 167 | // Text extracts the selection's ranges. The granularity is already encoded in | |
| @@ -63,8 +63,9 @@ func (s Selection) Ranges(doc *Document) []CellRange { | |||||
| 63 | return ranges | 63 | return ranges | ||
| 64 | } | 64 | } | ||
| 65 | 65 | ||||
| 66 | - | // RangeForRow returns the canonical inclusive range for one document row. It | 66 | + | // RangeForRow returns the canonical inclusive range for one document row. |
| 67 | - | // lets viewport renderers avoid materializing ranges for off-screen rows. | 67 | + | // End == Start-1 denotes a selected row with no cells. It lets viewport |
| 68 | + | // renderers avoid materializing ranges for off-screen rows. | |||
| 68 | func (s Selection) RangeForRow(doc *Document, row int) (CellRange, bool) { | 69 | func (s Selection) RangeForRow(doc *Document, row int) (CellRange, bool) { | ||
| 69 | bounds, ok := s.bounds(doc) | 70 | bounds, ok := s.bounds(doc) | ||
| 70 | if !ok { | 71 | if !ok { | ||
| @@ -120,10 +121,16 @@ func (b selectionBounds) rangeForRow(doc *Document, row int) (CellRange, bool) { | |||||
| 120 | return CellRange{}, false | 121 | return CellRange{}, false | ||
| 121 | } | 122 | } | ||
| 122 | cells := doc.Row(row) | 123 | cells := doc.Row(row) | ||
| 124 | + | return b.rangeForCells(row, cells, contentEndOf(cells, doc.snapshot.Bound(row))) | |||
| 125 | + | } | |||
| 126 | + | ||||
| 127 | + | func (b selectionBounds) rangeForCells(row int, cells []renderer.Cell, limit int) (CellRange, bool) { | |||
| 128 | + | if row < b.start.Row || row > b.end.Row { | |||
| 129 | + | return CellRange{}, false | |||
| 130 | + | } | |||
| 123 | if len(cells) == 0 { | 131 | if len(cells) == 0 { | ||
| 124 | return CellRange{Row: row}, true | 132 | return CellRange{Row: row}, true | ||
| 125 | } | 133 | } | ||
| 126 | - | limit := doc.contentEnd(row) | |||
| 127 | if limit == 0 { | 134 | if limit == 0 { | ||
| 128 | return CellRange{Row: row, End: -1}, true | 135 | return CellRange{Row: row, End: -1}, true | ||
| 129 | } | 136 | } | ||
| @@ -147,11 +154,14 @@ func (b selectionBounds) rangeForRow(doc *Document, row int) (CellRange, bool) { | |||||
| 147 | 154 | ||||
| 148 | // newlineMarker reports the column marking a selected hard newline on row. | 155 | // newlineMarker reports the column marking a selected hard newline on row. | ||
| 149 | func (b selectionBounds) newlineMarker(doc *Document, row int) (int, bool) { | 156 | func (b selectionBounds) newlineMarker(doc *Document, row int) (int, bool) { | ||
| 150 | - | if row < b.start.Row || row > b.end.Row || doc.snapshot.Bound(row).Soft || (!b.linewise && row == b.end.Row) { | 157 | + | return b.newlineMarkerAt(row, doc.contentEnd(row), doc.Width(), doc.snapshot.Bound(row).Soft) |
| 158 | + | } | |||
| 159 | + | ||||
| 160 | + | func (b selectionBounds) newlineMarkerAt(row, end, width int, soft bool) (int, bool) { | |||
| 161 | + | if row < b.start.Row || row > b.end.Row || soft || (!b.linewise && row == b.end.Row) { | |||
| 151 | return 0, false | 162 | return 0, false | ||
| 152 | } | 163 | } | ||
| 153 | - | end := doc.contentEnd(row) | 164 | + | return end, end > 0 && end < width |
| 154 | - | return end, end > 0 && end < doc.Width() | |||
| 155 | } | 165 | } | ||
| 156 | 166 | ||||
| 157 | // Text extracts the selection's ranges. The granularity is already encoded in | 167 | // Text extracts the selection's ranges. The granularity is already encoded in | ||