detached b268ad5

fix(copy): harden search and release handling

commit
b268ad5171878004207cdb56e1aaa354853909e9
author
brice · · 2026-07-15 10:57 UTC
parent
07e48e3
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +86 -22
M internal/usecase/copy/mode.go +9 -1
@@ -234,6 +234,9 @@ func (m *Mode) ExtendWordSelection(p Pos) bool {
234234 return true
235235 }
236236 func (m *Mode) Search(query string) bool {
237+ if m == nil {
238+ return false
239+ }
237240 return m.SetSearchMatches(query, FindMatches(m.document, query), 0)
238241 }
239242 func (m *Mode) SetSearchMatches(query string, matches []SearchMatch, index int) bool {
@@ -267,7 +270,12 @@ func (m *Mode) moveToSearchMatch(match SearchMatch) {
267270 if m == nil || m.document == nil {
268271 return
269272 }
270- if m.navigator.Set(m.document, Pos{Row: match.Row, Col: match.Start}) && m.selection.Enabled {
273+ pos, ok := m.document.Normalize(Pos{Row: match.Row, Col: match.Start})
274+ if !ok {
275+ return
276+ }
277+ m.navigator.Set(m.document, pos)
278+ if m.selection.Enabled {
271279 m.selection.Extend(m.navigator.Pos)
272280 }
273281 m.adjustViewport()
@@ -234,6 +234,9 @@ func (m *Mode) ExtendWordSelection(p Pos) bool {
234 return true234 return true
235 }235 }
236 func (m *Mode) Search(query string) bool {236 func (m *Mode) Search(query string) bool {
237+ if m == nil {
238+ return false
239+ }
237 return m.SetSearchMatches(query, FindMatches(m.document, query), 0)240 return m.SetSearchMatches(query, FindMatches(m.document, query), 0)
238 }241 }
239 func (m *Mode) SetSearchMatches(query string, matches []SearchMatch, index int) bool {242 func (m *Mode) SetSearchMatches(query string, matches []SearchMatch, index int) bool {
@@ -267,7 +270,12 @@ func (m *Mode) moveToSearchMatch(match SearchMatch) {
267 if m == nil || m.document == nil {270 if m == nil || m.document == nil {
268 return271 return
269 }272 }
270- if m.navigator.Set(m.document, Pos{Row: match.Row, Col: match.Start}) && m.selection.Enabled {273+ pos, ok := m.document.Normalize(Pos{Row: match.Row, Col: match.Start})
274+ if !ok {
275+ return
276+ }
277+ m.navigator.Set(m.document, pos)
278+ if m.selection.Enabled {
271 m.selection.Extend(m.navigator.Pos)279 m.selection.Extend(m.navigator.Pos)
272 }280 }
273 m.adjustViewport()281 m.adjustViewport()
M internal/usecase/copy/mode_test.go +15 -0
@@ -92,6 +92,21 @@ func TestCopyModeSearchNavigationExtendsActiveSelection(t *testing.T) {
9292 require.Equal(t, Selection{Anchor: Pos{Row: 0}, Active: Pos{Row: 2, Col: 2}, Granularity: Character, Enabled: true}, m.Selection())
9393 }
9494
95+func TestCopyModeSearchExtendsSelectionWhenMatchIsAlreadyAtCursor(t *testing.T) {
96+ m := modeFor([]string{"zero", "one"}, 2)
97+ require.True(t, m.StartCharacterSelection(Pos{Row: 0}))
98+ require.True(t, m.ExtendCharacterSelection(Pos{Row: 1, Col: 1}))
99+ require.True(t, m.SetPosition(Pos{Row: 0}))
100+
101+ require.True(t, m.SetSearchMatches("zero", []SearchMatch{{Row: 0, Start: 0, End: 4}}, 0))
102+ require.Equal(t, Pos{Row: 0}, m.Selection().Active)
103+}
104+
105+func TestCopyModeSearchNilMode(t *testing.T) {
106+ var mode *Mode
107+ require.False(t, mode.Search("anything"))
108+}
109+
95110 func TestCopyModeRenderBoundsCursorToRenderedRow(t *testing.T) {
96111 m := NewMode(NewDocument(NewSnapshotFromRows([][]renderer.Cell{row("alpha")}, 2, 1), ""))
97112 require.True(t, m.SetPosition(Pos{Row: 0, Col: 4}), "cursor remains valid in the document despite a narrow render frame")
@@ -92,6 +92,21 @@ func TestCopyModeSearchNavigationExtendsActiveSelection(t *testing.T) {
92 require.Equal(t, Selection{Anchor: Pos{Row: 0}, Active: Pos{Row: 2, Col: 2}, Granularity: Character, Enabled: true}, m.Selection())92 require.Equal(t, Selection{Anchor: Pos{Row: 0}, Active: Pos{Row: 2, Col: 2}, Granularity: Character, Enabled: true}, m.Selection())
93 }93 }
94 94
95+func TestCopyModeSearchExtendsSelectionWhenMatchIsAlreadyAtCursor(t *testing.T) {
96+ m := modeFor([]string{"zero", "one"}, 2)
97+ require.True(t, m.StartCharacterSelection(Pos{Row: 0}))
98+ require.True(t, m.ExtendCharacterSelection(Pos{Row: 1, Col: 1}))
99+ require.True(t, m.SetPosition(Pos{Row: 0}))
100+
101+ require.True(t, m.SetSearchMatches("zero", []SearchMatch{{Row: 0, Start: 0, End: 4}}, 0))
102+ require.Equal(t, Pos{Row: 0}, m.Selection().Active)
103+}
104+
105+func TestCopyModeSearchNilMode(t *testing.T) {
106+ var mode *Mode
107+ require.False(t, mode.Search("anything"))
108+}
109+
95 func TestCopyModeRenderBoundsCursorToRenderedRow(t *testing.T) {110 func TestCopyModeRenderBoundsCursorToRenderedRow(t *testing.T) {
96 m := NewMode(NewDocument(NewSnapshotFromRows([][]renderer.Cell{row("alpha")}, 2, 1), ""))111 m := NewMode(NewDocument(NewSnapshotFromRows([][]renderer.Cell{row("alpha")}, 2, 1), ""))
97 require.True(t, m.SetPosition(Pos{Row: 0, Col: 4}), "cursor remains valid in the document despite a narrow render frame")112 require.True(t, m.SetPosition(Pos{Row: 0, Col: 4}), "cursor remains valid in the document despite a narrow render frame")
M internal/usecase/daemon/copy_mouse_input.go +1 -3
@@ -23,9 +23,7 @@ func (d *Daemon) handleCopyMouse(sess *session, ac *attachedClient, tb *tab, ev
2323 return true
2424 }
2525 if ev.Button == mouse.Left && ev.Type == mouse.Release {
26- rt.copyMu.Lock()
27- rt.invalidateCopyPointerLocked(false)
28- rt.copyMu.Unlock()
26+ d.handleActiveCopyMouse(sess, ac, tb, ev)
2927 return true
3028 }
3129 switch ev.Button {
@@ -23,9 +23,7 @@ func (d *Daemon) handleCopyMouse(sess *session, ac *attachedClient, tb *tab, ev
23 return true23 return true
24 }24 }
25 if ev.Button == mouse.Left && ev.Type == mouse.Release {25 if ev.Button == mouse.Left && ev.Type == mouse.Release {
26- rt.copyMu.Lock()26+ d.handleActiveCopyMouse(sess, ac, tb, ev)
27- rt.invalidateCopyPointerLocked(false)
28- rt.copyMu.Unlock()
29 return true27 return true
30 }28 }
31 switch ev.Button {29 switch ev.Button {
M internal/usecase/daemon/copymode_lifecycle_test.go +36 -4
@@ -208,16 +208,32 @@ func TestCopyModeLifecyclePublicationEpochRejectsReleaseAndPaneClose(t *testing.
208208 close(reached)
209209 <-resume
210210 }
211+ resumed := false
212+ defer func() {
213+ if !resumed {
214+ close(resume)
215+ }
216+ d.beforeCopyModeRevalidate = nil
217+ }()
211218 done := make(chan struct{})
212219 go func() {
213220 d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
214221 close(done)
215222 }()
216- <-reached
223+ select {
224+ case <-reached:
225+ case <-time.After(time.Second):
226+ t.Fatal("copy mode publication did not reach revalidation gate")
227+ }
217228
218229 tc.invalidate(t, d, sess, ac, tb, p)
219230 close(resume)
220- <-done
231+ resumed = true
232+ select {
233+ case <-done:
234+ case <-time.After(time.Second):
235+ t.Fatal("copy mode publication did not finish after revalidation gate was released")
236+ }
221237
222238 ac.overlays.copyMu.Lock()
223239 require.Nil(t, ac.overlays.copyMode, "stale publication must not resurrect copy mode")
@@ -257,15 +273,31 @@ func TestCopyModeLifecycleFloatingCloseDuringPublicationDoesNotResurrect(t *test
257273 close(reached)
258274 <-resume
259275 }
276+ resumed := false
277+ defer func() {
278+ if !resumed {
279+ close(resume)
280+ }
281+ d.beforeCopyModeRevalidate = nil
282+ }()
260283 done := make(chan struct{})
261284 go func() {
262285 d.handleInput(sess, ac, motion)
263286 close(done)
264287 }()
265- <-reached
288+ select {
289+ case <-reached:
290+ case <-time.After(time.Second):
291+ t.Fatal("copy mode publication did not reach revalidation gate")
292+ }
266293 d.teardownFloating(tb, ac)
267294 close(resume)
268- <-done
295+ resumed = true
296+ select {
297+ case <-done:
298+ case <-time.After(time.Second):
299+ t.Fatal("copy mode publication did not finish after revalidation gate was released")
300+ }
269301
270302 ac.overlays.copyMu.Lock()
271303 require.Nil(t, ac.overlays.copyMode)
@@ -208,16 +208,32 @@ func TestCopyModeLifecyclePublicationEpochRejectsReleaseAndPaneClose(t *testing.
208 close(reached)208 close(reached)
209 <-resume209 <-resume
210 }210 }
211+ resumed := false
212+ defer func() {
213+ if !resumed {
214+ close(resume)
215+ }
216+ d.beforeCopyModeRevalidate = nil
217+ }()
211 done := make(chan struct{})218 done := make(chan struct{})
212 go func() {219 go func() {
213 d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))220 d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
214 close(done)221 close(done)
215 }()222 }()
216- <-reached223+ select {
224+ case <-reached:
225+ case <-time.After(time.Second):
226+ t.Fatal("copy mode publication did not reach revalidation gate")
227+ }
217 228
218 tc.invalidate(t, d, sess, ac, tb, p)229 tc.invalidate(t, d, sess, ac, tb, p)
219 close(resume)230 close(resume)
220- <-done231+ resumed = true
232+ select {
233+ case <-done:
234+ case <-time.After(time.Second):
235+ t.Fatal("copy mode publication did not finish after revalidation gate was released")
236+ }
221 237
222 ac.overlays.copyMu.Lock()238 ac.overlays.copyMu.Lock()
223 require.Nil(t, ac.overlays.copyMode, "stale publication must not resurrect copy mode")239 require.Nil(t, ac.overlays.copyMode, "stale publication must not resurrect copy mode")
@@ -257,15 +273,31 @@ func TestCopyModeLifecycleFloatingCloseDuringPublicationDoesNotResurrect(t *test
257 close(reached)273 close(reached)
258 <-resume274 <-resume
259 }275 }
276+ resumed := false
277+ defer func() {
278+ if !resumed {
279+ close(resume)
280+ }
281+ d.beforeCopyModeRevalidate = nil
282+ }()
260 done := make(chan struct{})283 done := make(chan struct{})
261 go func() {284 go func() {
262 d.handleInput(sess, ac, motion)285 d.handleInput(sess, ac, motion)
263 close(done)286 close(done)
264 }()287 }()
265- <-reached288+ select {
289+ case <-reached:
290+ case <-time.After(time.Second):
291+ t.Fatal("copy mode publication did not reach revalidation gate")
292+ }
266 d.teardownFloating(tb, ac)293 d.teardownFloating(tb, ac)
267 close(resume)294 close(resume)
268- <-done295+ resumed = true
296+ select {
297+ case <-done:
298+ case <-time.After(time.Second):
299+ t.Fatal("copy mode publication did not finish after revalidation gate was released")
300+ }
269 301
270 ac.overlays.copyMu.Lock()302 ac.overlays.copyMu.Lock()
271 require.Nil(t, ac.overlays.copyMode)303 require.Nil(t, ac.overlays.copyMode)
M internal/usecase/daemon/input_test.go +25 -14
@@ -1140,31 +1140,24 @@ func TestCopyModeStatusRowPressClearsDragState(t *testing.T) {
11401140 require.Equal(t, 1, hi, "status-row press must invalidate the drag so the motion is a no-op")
11411141 }
11421142
1143-// TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset covers a
1144-// regression where the first drag Motion extended the selection using the
1145-// scrollback length captured at Press time instead of the current
1146-// scrollback length, so if lines were evicted into scrollback between the
1147-// Press and the first Motion, the extend target landed on the wrong row.
1148-func TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset(t *testing.T) {
1143+// TestMouseNormalScreenDragUsesPressOwnedDocumentAfterOutputEviction verifies
1144+// that output eviction after Press does not change endpoint mapping: the
1145+// immutable press-owned Document keeps pointer rows 0→1 mapped to rows 0→1.
1146+func TestMouseNormalScreenDragUsesPressOwnedDocumentAfterOutputEviction(t *testing.T) {
11491147 p, _ := newBlockingPTY(t)
11501148 d, sess, ac, _ := newManualSessionWithPTYs(t, p)
11511149 installTestHistory(sess.tabs[0].focusedPane(), vt.HistoryConfig{MaxRows: 50})
11521150 require.Equal(t, 23, sess.tabs[0].focusedPane().screen.Frame.Height, "fixture assumption: status row is wire row 24")
11531151
1154- // Press on a content row while scrollback is empty: the anchor is
1155- // content-stable at the first body row = document row 0.
1152+ // Press anchors document row 0 before output eviction.
11561153 d.handleInput(sess, ac, []byte("\x1b[<0;1;2M"))
11571154
1158- // Simulate 5 lines evicted into scrollback between the Press and the
1159- // first Motion (e.g. the child kept producing output).
1155+ // Output evicts five lines after Press; the Document remains immutable.
11601156 for range 5 {
11611157 sess.tabs[0].focusedPane().history.Append(testRow("evicted"))
11621158 }
11631159
1164- // Motion lands on wire row 3 (0-based row 2 of the *current* screen).
1165- // The extend target must track the pointer's current content row:
1166- // current scrollback length (5) + ev.Row (2) = 7 -- not the stale
1167- // pressTop(0)+ev.Row(2) = 2.
1160+ // Motion maps the next endpoint through the press-owned Document.
11681161 d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
11691162
11701163 require.NotNil(t, ac.overlays.copyMode)
@@ -1511,6 +1504,24 @@ func TestActiveCopyPressOnOtherPaneFocusesAndExitsBeforeFutureInput(t *testing.T
15111504 require.Same(t, p2, pointer.pane)
15121505 }
15131506
1507+func TestCopyModeReleaseMapsFinalEndpointAndInvalidatesPointer(t *testing.T) {
1508+ d, sess, ac, _ := mouseCopyHarness(t, "alpha bravo")
1509+
1510+ // Motion starts an active drag; Release then arrives farther away without
1511+ // another Motion and must still extend its final endpoint.
1512+ copyMouseInput(d, sess, ac, "\x1b[<0;1;2M\x1b[<32;5;2M")
1513+ copyMouseInput(d, sess, ac, "\x1b[<0;8;2m")
1514+
1515+ ac.overlays.copyMu.Lock()
1516+ selection := ac.overlays.copyMode.Selection()
1517+ pointerValid := ac.overlays.copyPointer.valid
1518+ ac.overlays.copyMu.Unlock()
1519+ require.True(t, selection.Enabled)
1520+ require.Equal(t, scopy.Pos{Row: 0, Col: 0}, selection.Anchor)
1521+ require.Equal(t, scopy.Pos{Row: 0, Col: 7}, selection.Active)
1522+ require.False(t, pointerValid)
1523+}
1524+
15141525 func TestCopySearchReleaseMapsFinalEndpointAndInvalidatesPointer(t *testing.T) {
15151526 for _, tc := range []struct {
15161527 name string
@@ -1140,31 +1140,24 @@ func TestCopyModeStatusRowPressClearsDragState(t *testing.T) {
1140 require.Equal(t, 1, hi, "status-row press must invalidate the drag so the motion is a no-op")1140 require.Equal(t, 1, hi, "status-row press must invalidate the drag so the motion is a no-op")
1141 }1141 }
1142 1142
1143-// TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset covers a1143+// TestMouseNormalScreenDragUsesPressOwnedDocumentAfterOutputEviction verifies
1144-// regression where the first drag Motion extended the selection using the1144+// that output eviction after Press does not change endpoint mapping: the
1145-// scrollback length captured at Press time instead of the current1145+// immutable press-owned Document keeps pointer rows 0→1 mapped to rows 0→1.
1146-// scrollback length, so if lines were evicted into scrollback between the1146+func TestMouseNormalScreenDragUsesPressOwnedDocumentAfterOutputEviction(t *testing.T) {
1147-// Press and the first Motion, the extend target landed on the wrong row.
1148-func TestMouseNormalScreenDragExtendsToCurrentScrollbackOffset(t *testing.T) {
1149 p, _ := newBlockingPTY(t)1147 p, _ := newBlockingPTY(t)
1150 d, sess, ac, _ := newManualSessionWithPTYs(t, p)1148 d, sess, ac, _ := newManualSessionWithPTYs(t, p)
1151 installTestHistory(sess.tabs[0].focusedPane(), vt.HistoryConfig{MaxRows: 50})1149 installTestHistory(sess.tabs[0].focusedPane(), vt.HistoryConfig{MaxRows: 50})
1152 require.Equal(t, 23, sess.tabs[0].focusedPane().screen.Frame.Height, "fixture assumption: status row is wire row 24")1150 require.Equal(t, 23, sess.tabs[0].focusedPane().screen.Frame.Height, "fixture assumption: status row is wire row 24")
1153 1151
1154- // Press on a content row while scrollback is empty: the anchor is1152+ // Press anchors document row 0 before output eviction.
1155- // content-stable at the first body row = document row 0.
1156 d.handleInput(sess, ac, []byte("\x1b[<0;1;2M"))1153 d.handleInput(sess, ac, []byte("\x1b[<0;1;2M"))
1157 1154
1158- // Simulate 5 lines evicted into scrollback between the Press and the1155+ // Output evicts five lines after Press; the Document remains immutable.
1159- // first Motion (e.g. the child kept producing output).
1160 for range 5 {1156 for range 5 {
1161 sess.tabs[0].focusedPane().history.Append(testRow("evicted"))1157 sess.tabs[0].focusedPane().history.Append(testRow("evicted"))
1162 }1158 }
1163 1159
1164- // Motion lands on wire row 3 (0-based row 2 of the *current* screen).1160+ // Motion maps the next endpoint through the press-owned Document.
1165- // The extend target must track the pointer's current content row:
1166- // current scrollback length (5) + ev.Row (2) = 7 -- not the stale
1167- // pressTop(0)+ev.Row(2) = 2.
1168 d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))1161 d.handleInput(sess, ac, []byte("\x1b[<32;1;3M"))
1169 1162
1170 require.NotNil(t, ac.overlays.copyMode)1163 require.NotNil(t, ac.overlays.copyMode)
@@ -1511,6 +1504,24 @@ func TestActiveCopyPressOnOtherPaneFocusesAndExitsBeforeFutureInput(t *testing.T
1511 require.Same(t, p2, pointer.pane)1504 require.Same(t, p2, pointer.pane)
1512 }1505 }
1513 1506
1507+func TestCopyModeReleaseMapsFinalEndpointAndInvalidatesPointer(t *testing.T) {
1508+ d, sess, ac, _ := mouseCopyHarness(t, "alpha bravo")
1509+
1510+ // Motion starts an active drag; Release then arrives farther away without
1511+ // another Motion and must still extend its final endpoint.
1512+ copyMouseInput(d, sess, ac, "\x1b[<0;1;2M\x1b[<32;5;2M")
1513+ copyMouseInput(d, sess, ac, "\x1b[<0;8;2m")
1514+
1515+ ac.overlays.copyMu.Lock()
1516+ selection := ac.overlays.copyMode.Selection()
1517+ pointerValid := ac.overlays.copyPointer.valid
1518+ ac.overlays.copyMu.Unlock()
1519+ require.True(t, selection.Enabled)
1520+ require.Equal(t, scopy.Pos{Row: 0, Col: 0}, selection.Anchor)
1521+ require.Equal(t, scopy.Pos{Row: 0, Col: 7}, selection.Active)
1522+ require.False(t, pointerValid)
1523+}
1524+
1514 func TestCopySearchReleaseMapsFinalEndpointAndInvalidatesPointer(t *testing.T) {1525 func TestCopySearchReleaseMapsFinalEndpointAndInvalidatesPointer(t *testing.T) {
1515 for _, tc := range []struct {1526 for _, tc := range []struct {
1516 name string1527 name string

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close