detached b23180e

refactor(history): clarify snapshot ownership

commit
b23180e62994ab3b761ad9025d3271af1b115cc8
author
brice · · 2026-07-13 04:54 UTC
parent
7913f91
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
6 files · +23 -40
Split
UnifiedSplit
M internal/usecase/copy/mode.go +2 -1
@@ -53,7 +53,8 @@ func NewSnapshotFromRows(rows [][]renderer.Cell, width, height int) Snapshot {
5353 // Len returns the number of document rows.
5454 func (s Snapshot) Len() int { return s.history.Len() + s.screen.Height }
5555
56-// Row returns document row i, or nil when i is out of range.
56+// Row returns document row i, or nil when i is out of range. History rows
57+// borrow immutable history storage and must be treated as read-only.
5758 func (s Snapshot) Row(i int) []renderer.Cell {
5859 if i < 0 {
5960 return nil
@@ -53,7 +53,8 @@ func NewSnapshotFromRows(rows [][]renderer.Cell, width, height int) Snapshot {
53 // Len returns the number of document rows.53 // Len returns the number of document rows.
54 func (s Snapshot) Len() int { return s.history.Len() + s.screen.Height }54 func (s Snapshot) Len() int { return s.history.Len() + s.screen.Height }
55 55
56-// Row returns document row i, or nil when i is out of range.56+// Row returns document row i, or nil when i is out of range. History rows
57+// borrow immutable history storage and must be treated as read-only.
57 func (s Snapshot) Row(i int) []renderer.Cell {58 func (s Snapshot) Row(i int) []renderer.Cell {
58 if i < 0 {59 if i < 0 {
59 return nil60 return nil
M internal/usecase/copy/mode_test.go +3 -3
@@ -610,9 +610,9 @@ func BenchmarkNewSnapshot10KRows(b *testing.B) {
610610 historyRows = 10_000
611611 )
612612
613- scrollback := newHistory(historyRows)
613+ history := newHistory(historyRows)
614614 for range historyRows {
615- scrollback.Append(row(strings.Repeat("x", width)))
615+ history.Append(row(strings.Repeat("x", width)))
616616 }
617617 screen := renderer.NewFrame(width, height)
618618 for y := range height {
@@ -622,7 +622,7 @@ func BenchmarkNewSnapshot10KRows(b *testing.B) {
622622 b.ReportAllocs()
623623 b.ResetTimer()
624624 for b.Loop() {
625- benchmarkSnapshotSink = NewSnapshot(scrollback, screen)
625+ benchmarkSnapshotSink = NewSnapshot(history, screen)
626626 }
627627 }
628628
@@ -610,9 +610,9 @@ func BenchmarkNewSnapshot10KRows(b *testing.B) {
610 historyRows = 10_000610 historyRows = 10_000
611 )611 )
612 612
613- scrollback := newHistory(historyRows)613+ history := newHistory(historyRows)
614 for range historyRows {614 for range historyRows {
615- scrollback.Append(row(strings.Repeat("x", width)))615+ history.Append(row(strings.Repeat("x", width)))
616 }616 }
617 screen := renderer.NewFrame(width, height)617 screen := renderer.NewFrame(width, height)
618 for y := range height {618 for y := range height {
@@ -622,7 +622,7 @@ func BenchmarkNewSnapshot10KRows(b *testing.B) {
622 b.ReportAllocs()622 b.ReportAllocs()
623 b.ResetTimer()623 b.ResetTimer()
624 for b.Loop() {624 for b.Loop() {
625- benchmarkSnapshotSink = NewSnapshot(scrollback, screen)625+ benchmarkSnapshotSink = NewSnapshot(history, screen)
626 }626 }
627 }627 }
628 628
M internal/usecase/daemon/daemon.go +1 -1
@@ -373,7 +373,7 @@ func (d *Daemon) Serve(ctx context.Context, l ports.Listener) error {
373373 })
374374 }
375375 if d.snapsEnabled {
376- d.startSnapshotEncodeWorker(d.serveCtx)
376+ d.startSnapshotEncodeWorker()
377377 d.sessWg.Go(func() {
378378 d.snapshotSaver(d.serveCtx)
379379 })
@@ -373,7 +373,7 @@ func (d *Daemon) Serve(ctx context.Context, l ports.Listener) error {
373 })373 })
374 }374 }
375 if d.snapsEnabled {375 if d.snapsEnabled {
376- d.startSnapshotEncodeWorker(d.serveCtx)376+ d.startSnapshotEncodeWorker()
377 d.sessWg.Go(func() {377 d.sessWg.Go(func() {
378 d.snapshotSaver(d.serveCtx)378 d.snapshotSaver(d.serveCtx)
379 })379 })
M internal/usecase/daemon/performance_benchmark_test.go +2 -7
@@ -1,7 +1,6 @@
11 package daemon
22
33 import (
4- "context"
54 "fmt"
65 "io"
76 "runtime"
@@ -736,12 +735,8 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF
736735 resizeSizes: [2]domain.Size{{Cols: 100, Rows: 30}, config.size},
737736 }
738737 WithSnapshotStore(fixture.snaps)(d)
739- snapshotCtx, cancelSnapshotWorker := context.WithCancel(context.Background())
740- d.startSnapshotEncodeWorker(snapshotCtx)
741- t.Cleanup(func() {
742- cancelSnapshotWorker()
743- d.stopSnapshotEncodeWorker()
744- })
738+ d.startSnapshotEncodeWorker()
739+ t.Cleanup(d.stopSnapshotEncodeWorker)
745740 for tabIndex, tb := range sess.tabs {
746741 fixture.configureTab(tb, tabIndex, config.panes, config.historyRows, tabSize(config.size))
747742 }
@@ -1,7 +1,6 @@
1 package daemon1 package daemon
2 2
3 import (3 import (
4- "context"
5 "fmt"4 "fmt"
6 "io"5 "io"
7 "runtime"6 "runtime"
@@ -736,12 +735,8 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF
736 resizeSizes: [2]domain.Size{{Cols: 100, Rows: 30}, config.size},735 resizeSizes: [2]domain.Size{{Cols: 100, Rows: 30}, config.size},
737 }736 }
738 WithSnapshotStore(fixture.snaps)(d)737 WithSnapshotStore(fixture.snaps)(d)
739- snapshotCtx, cancelSnapshotWorker := context.WithCancel(context.Background())738+ d.startSnapshotEncodeWorker()
740- d.startSnapshotEncodeWorker(snapshotCtx)739+ t.Cleanup(d.stopSnapshotEncodeWorker)
741- t.Cleanup(func() {
742- cancelSnapshotWorker()
743- d.stopSnapshotEncodeWorker()
744- })
745 for tabIndex, tb := range sess.tabs {740 for tabIndex, tb := range sess.tabs {
746 fixture.configureTab(tb, tabIndex, config.panes, config.historyRows, tabSize(config.size))741 fixture.configureTab(tb, tabIndex, config.panes, config.historyRows, tabSize(config.size))
747 }742 }
M internal/usecase/daemon/snapshot.go +2 -2
@@ -504,8 +504,8 @@ func (d *Daemon) finishSnapshotCapture(capture *snapshotCapture, succeeded bool)
504504 })
505505 }
506506
507-func (d *Daemon) startSnapshotEncodeWorker(ctx context.Context) {
508- if d == nil || ctx.Err() != nil {
507+func (d *Daemon) startSnapshotEncodeWorker() {
508+ if d == nil {
509509 return
510510 }
511511 d.snapshotWorkerMu.Lock()
@@ -504,8 +504,8 @@ func (d *Daemon) finishSnapshotCapture(capture *snapshotCapture, succeeded bool)
504 })504 })
505 }505 }
506 506
507-func (d *Daemon) startSnapshotEncodeWorker(ctx context.Context) {507+func (d *Daemon) startSnapshotEncodeWorker() {
508- if d == nil || ctx.Err() != nil {508+ if d == nil {
509 return509 return
510 }510 }
511 d.snapshotWorkerMu.Lock()511 d.snapshotWorkerMu.Lock()
M internal/usecase/daemon/snapshot_test.go +13 -26
@@ -25,12 +25,8 @@ func TestSnapshotEncodeWorkerDefersMarshalUntilAfterCapture(t *testing.T) {
2525 store := &channelSnapshotStore{writes: make(chan []byte, 1)}
2626 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
2727 WithSnapshotStore(store)(d)
28- ctx, cancel := context.WithCancel(t.Context())
29- t.Cleanup(func() {
30- cancel()
31- d.stopSnapshotEncodeWorker()
32- })
33- d.startSnapshotEncodeWorker(ctx)
28+ t.Cleanup(d.stopSnapshotEncodeWorker)
29+ d.startSnapshotEncodeWorker()
3430
3531 entered := make(chan struct{})
3632 release := make(chan struct{})
@@ -72,11 +68,9 @@ func exerciseBlockedSnapshotWorker(t *testing.T, blockEncode bool) {
7268 store := newGatedSnapshotStore()
7369 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
7470 WithSnapshotStore(store)(d)
75- ctx, cancel := context.WithCancel(t.Context())
76- d.startSnapshotEncodeWorker(ctx)
71+ d.startSnapshotEncodeWorker()
7772 t.Cleanup(func() {
7873 store.unblock()
79- cancel()
8074 d.stopSnapshotEncodeWorker()
8175 })
8276 if blockEncode {
@@ -157,9 +151,7 @@ func TestFinalSnapshotFlushPersistsNewestNamedSessionBeforeWorkerTeardown(t *tes
157151 store := &channelSnapshotStore{writes: make(chan []byte, 1)}
158152 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clock)
159153 WithSnapshotStore(store)(d)
160- ctx, cancel := context.WithCancel(t.Context())
161- defer cancel()
162- d.startSnapshotEncodeWorker(ctx)
154+ d.startSnapshotEncodeWorker()
163155
164156 entered := make(chan struct{})
165157 release := make(chan struct{})
@@ -169,7 +161,8 @@ func TestFinalSnapshotFlushPersistsNewestNamedSessionBeforeWorkerTeardown(t *tes
169161 return snapcodec.Marshal(s)
170162 }
171163 sess := newSnapshotTestSession(t, "final", false, "/work")
172- pty := sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)
164+ pty, ok := sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)
165+ require.True(t, ok)
173166 pty.EXPECT().Close().Return(nil).Once()
174167 sess.tabs[0].panes["pane-1"].screen.Write([]byte("\rnewest"))
175168 d.sessions[sess.id] = sess
@@ -202,9 +195,7 @@ func TestStopSnapshotEncodeWorkerDoesNotWaitForUncancellableWrite(t *testing.T)
202195 clock := newFinalFlushClock()
203196 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clock)
204197 WithSnapshotStore(store)(d)
205- ctx, cancel := context.WithCancel(t.Context())
206- defer cancel()
207- d.startSnapshotEncodeWorker(ctx)
198+ d.startSnapshotEncodeWorker()
208199
209200 sess := newSnapshotTestSession(t, "stuck-write", false, "/work")
210201 require.True(t, d.captureSession(sess))
@@ -240,10 +231,8 @@ func TestStoppedSnapshotWorkerLeavesCaptureRetryable(t *testing.T) {
240231 store := &channelSnapshotStore{writes: make(chan []byte, 1)}
241232 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
242233 WithSnapshotStore(store)(d)
243- ctx, cancel := context.WithCancel(t.Context())
244- d.startSnapshotEncodeWorker(ctx)
234+ d.startSnapshotEncodeWorker()
245235 d.stopSnapshotEncodeWorker()
246- t.Cleanup(cancel)
247236
248237 sess := newSnapshotTestSession(t, "stopped", false, "/work")
249238 markSnapshotDirty(sess)
@@ -326,7 +315,9 @@ func TestNamedFinalSnapshotSurvivesSaturatedQueue(t *testing.T) {
326315 first := newSnapshotTestSession(t, "first", false, "/work")
327316 second := newSnapshotTestSession(t, "second", false, "/work")
328317 final := newSnapshotTestSession(t, "final", false, "/work")
329- final.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY).EXPECT().Close().Return(nil).Once()
318+ finalPTY, ok := final.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)
319+ require.True(t, ok)
320+ finalPTY.EXPECT().Close().Return(nil).Once()
330321 markSnapshotDirty(first)
331322 markSnapshotDirty(second)
332323 require.True(t, d.scheduleSnapshot(first))
@@ -915,12 +906,8 @@ func awaitSnapshotClean(t *testing.T, sess *session) {
915906
916907 func startSnapshotEncodeWorker(t *testing.T, d *Daemon) {
917908 t.Helper()
918- ctx, cancel := context.WithCancel(t.Context())
919- d.startSnapshotEncodeWorker(ctx)
920- t.Cleanup(func() {
921- cancel()
922- d.stopSnapshotEncodeWorker()
923- })
909+ d.startSnapshotEncodeWorker()
910+ t.Cleanup(d.stopSnapshotEncodeWorker)
924911 }
925912
926913 type gatedSnapshotStore struct {
@@ -25,12 +25,8 @@ func TestSnapshotEncodeWorkerDefersMarshalUntilAfterCapture(t *testing.T) {
25 store := &channelSnapshotStore{writes: make(chan []byte, 1)}25 store := &channelSnapshotStore{writes: make(chan []byte, 1)}
26 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})26 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
27 WithSnapshotStore(store)(d)27 WithSnapshotStore(store)(d)
28- ctx, cancel := context.WithCancel(t.Context())28+ t.Cleanup(d.stopSnapshotEncodeWorker)
29- t.Cleanup(func() {29+ d.startSnapshotEncodeWorker()
30- cancel()
31- d.stopSnapshotEncodeWorker()
32- })
33- d.startSnapshotEncodeWorker(ctx)
34 30
35 entered := make(chan struct{})31 entered := make(chan struct{})
36 release := make(chan struct{})32 release := make(chan struct{})
@@ -72,11 +68,9 @@ func exerciseBlockedSnapshotWorker(t *testing.T, blockEncode bool) {
72 store := newGatedSnapshotStore()68 store := newGatedSnapshotStore()
73 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})69 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
74 WithSnapshotStore(store)(d)70 WithSnapshotStore(store)(d)
75- ctx, cancel := context.WithCancel(t.Context())71+ d.startSnapshotEncodeWorker()
76- d.startSnapshotEncodeWorker(ctx)
77 t.Cleanup(func() {72 t.Cleanup(func() {
78 store.unblock()73 store.unblock()
79- cancel()
80 d.stopSnapshotEncodeWorker()74 d.stopSnapshotEncodeWorker()
81 })75 })
82 if blockEncode {76 if blockEncode {
@@ -157,9 +151,7 @@ func TestFinalSnapshotFlushPersistsNewestNamedSessionBeforeWorkerTeardown(t *tes
157 store := &channelSnapshotStore{writes: make(chan []byte, 1)}151 store := &channelSnapshotStore{writes: make(chan []byte, 1)}
158 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clock)152 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clock)
159 WithSnapshotStore(store)(d)153 WithSnapshotStore(store)(d)
160- ctx, cancel := context.WithCancel(t.Context())154+ d.startSnapshotEncodeWorker()
161- defer cancel()
162- d.startSnapshotEncodeWorker(ctx)
163 155
164 entered := make(chan struct{})156 entered := make(chan struct{})
165 release := make(chan struct{})157 release := make(chan struct{})
@@ -169,7 +161,8 @@ func TestFinalSnapshotFlushPersistsNewestNamedSessionBeforeWorkerTeardown(t *tes
169 return snapcodec.Marshal(s)161 return snapcodec.Marshal(s)
170 }162 }
171 sess := newSnapshotTestSession(t, "final", false, "/work")163 sess := newSnapshotTestSession(t, "final", false, "/work")
172- pty := sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)164+ pty, ok := sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)
165+ require.True(t, ok)
173 pty.EXPECT().Close().Return(nil).Once()166 pty.EXPECT().Close().Return(nil).Once()
174 sess.tabs[0].panes["pane-1"].screen.Write([]byte("\rnewest"))167 sess.tabs[0].panes["pane-1"].screen.Write([]byte("\rnewest"))
175 d.sessions[sess.id] = sess168 d.sessions[sess.id] = sess
@@ -202,9 +195,7 @@ func TestStopSnapshotEncodeWorkerDoesNotWaitForUncancellableWrite(t *testing.T)
202 clock := newFinalFlushClock()195 clock := newFinalFlushClock()
203 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clock)196 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), clock)
204 WithSnapshotStore(store)(d)197 WithSnapshotStore(store)(d)
205- ctx, cancel := context.WithCancel(t.Context())198+ d.startSnapshotEncodeWorker()
206- defer cancel()
207- d.startSnapshotEncodeWorker(ctx)
208 199
209 sess := newSnapshotTestSession(t, "stuck-write", false, "/work")200 sess := newSnapshotTestSession(t, "stuck-write", false, "/work")
210 require.True(t, d.captureSession(sess))201 require.True(t, d.captureSession(sess))
@@ -240,10 +231,8 @@ func TestStoppedSnapshotWorkerLeavesCaptureRetryable(t *testing.T) {
240 store := &channelSnapshotStore{writes: make(chan []byte, 1)}231 store := &channelSnapshotStore{writes: make(chan []byte, 1)}
241 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})232 d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
242 WithSnapshotStore(store)(d)233 WithSnapshotStore(store)(d)
243- ctx, cancel := context.WithCancel(t.Context())234+ d.startSnapshotEncodeWorker()
244- d.startSnapshotEncodeWorker(ctx)
245 d.stopSnapshotEncodeWorker()235 d.stopSnapshotEncodeWorker()
246- t.Cleanup(cancel)
247 236
248 sess := newSnapshotTestSession(t, "stopped", false, "/work")237 sess := newSnapshotTestSession(t, "stopped", false, "/work")
249 markSnapshotDirty(sess)238 markSnapshotDirty(sess)
@@ -326,7 +315,9 @@ func TestNamedFinalSnapshotSurvivesSaturatedQueue(t *testing.T) {
326 first := newSnapshotTestSession(t, "first", false, "/work")315 first := newSnapshotTestSession(t, "first", false, "/work")
327 second := newSnapshotTestSession(t, "second", false, "/work")316 second := newSnapshotTestSession(t, "second", false, "/work")
328 final := newSnapshotTestSession(t, "final", false, "/work")317 final := newSnapshotTestSession(t, "final", false, "/work")
329- final.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY).EXPECT().Close().Return(nil).Once()318+ finalPTY, ok := final.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)
319+ require.True(t, ok)
320+ finalPTY.EXPECT().Close().Return(nil).Once()
330 markSnapshotDirty(first)321 markSnapshotDirty(first)
331 markSnapshotDirty(second)322 markSnapshotDirty(second)
332 require.True(t, d.scheduleSnapshot(first))323 require.True(t, d.scheduleSnapshot(first))
@@ -915,12 +906,8 @@ func awaitSnapshotClean(t *testing.T, sess *session) {
915 906
916 func startSnapshotEncodeWorker(t *testing.T, d *Daemon) {907 func startSnapshotEncodeWorker(t *testing.T, d *Daemon) {
917 t.Helper()908 t.Helper()
918- ctx, cancel := context.WithCancel(t.Context())909+ d.startSnapshotEncodeWorker()
919- d.startSnapshotEncodeWorker(ctx)910+ t.Cleanup(d.stopSnapshotEncodeWorker)
920- t.Cleanup(func() {
921- cancel()
922- d.stopSnapshotEncodeWorker()
923- })
924 }911 }
925 912
926 type gatedSnapshotStore struct {913 type gatedSnapshotStore struct {

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