detached 5361974

test(core): pin the layout buffer reuse contracts

Review follow-ups for the layout buffers:
- an empty settledLayout stays nil, so shownLayout falls back to the live
  layout until the next publish;
- the overview phase of the sent-scenes test also checks each scene against
  a fresh Layout() taken before the step (IDs, hidden, and rects once
  settled), so the overview's real layouts sharing a screen buffer fail it;
- layoutInto must build in the caller's buffer, and a result kept across the
  calls that use the workspace scratch stays unchanged;
- the layout variants assert they hold the feature they are named for (the
  stash variant had put its window on a hidden workspace);
- maps.Keys replaces the hand-written key iterator.
commit
5361974ae6a25854a86df656a949595109543fbd
author
brice · · 2026-10-05 03:49 UTC
parent
9cb58f8
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
1 file · +130 -23
M internal/core/layout_buffers_test.go +130 -23
@@ -1,6 +1,7 @@
11 package core
22
33 import (
4+ "maps"
45 "reflect"
56 "slices"
67 "testing"
@@ -49,9 +50,6 @@ func layoutVariants(t *testing.T) map[string]*Monitor {
4950
5051 m = base(OverflowScroll, 4)
5152 m.Current().Apply(ActionToggleWindowStash)
52- m.Current().Apply(ActionToggleWindowStash)
53- m.Focus(2)
54- m.Current().Apply(ActionToggleWindowStash)
5553 v["stash"] = m
5654
5755 m = base(OverflowScroll, 4)
@@ -76,6 +74,21 @@ func layoutVariants(t *testing.T) map[string]*Monitor {
7674 m.Apply(ActionFocusWorkspaceUp)
7775 m.switchOff = 0.4
7876 v["slide"] = m
77+
78+ // The variants must hold what they are named for, on the workspace on
79+ // screen (the one whose scratch the tests exercise).
80+ for name, has := range map[string]func(*Workspace) bool{
81+ "stash": func(w *Workspace) bool { return len(w.Stash) > 0 && len(w.Columns) > 0 },
82+ "floats": func(w *Workspace) bool { return len(w.Floats) == 2 && len(w.Columns) > 0 },
83+ "shares": func(w *Workspace) bool { return validShares(w.Columns[0]) },
84+ "fixed-expanded": func(w *Workspace) bool { return w.Columns[1].Expanded },
85+ "fullscreen": func(w *Workspace) bool { return w.fullscreen == 2 },
86+ "scroll": func(w *Workspace) bool { return len(w.Columns) == 5 },
87+ } {
88+ if w := v[name].Current(); len(w.Columns) == 0 || !has(w) {
89+ t.Fatalf("variant %s: the workspace on screen lacks its feature: %+v", name, w.Columns)
90+ }
91+ }
7992 return v
8093 }
8194
@@ -85,7 +98,7 @@ func layoutVariants(t *testing.T) map[string]*Monitor {
8598 func TestLayoutIntoMatchesLayout(t *testing.T) {
8699 variants := layoutVariants(t)
87100 var buf []Placement
88- for _, name := range slices.Sorted(mapsKeys(variants)) {
101+ for _, name := range slices.Sorted(maps.Keys(variants)) {
89102 m := variants[name]
90103 want := m.Layout()
91104 if len(want) == 0 {
@@ -113,16 +126,6 @@ func TestLayoutIntoMatchesLayout(t *testing.T) {
113126 }
114127 }
115128
116-func mapsKeys[K comparable, V any](m map[K]V) func(func(K) bool) {
117- return func(yield func(K) bool) {
118- for k := range m {
119- if !yield(k) {
120- return
121- }
122- }
123- }
124-}
125-
126129 // TestLayoutResultsDoNotAlias: Layout() returns a slice the caller owns. A
127130 // retained result stays unchanged after further layouts of the same monitor
128131 // or workspace (nested or later, reused-buffer calls included), and two
@@ -145,6 +148,32 @@ func TestLayoutResultsDoNotAlias(t *testing.T) {
145148 m.Current().previewTiles()
146149 m.Current().overviewZoom()
147150 }
151+ // layoutInto builds in the caller's buffer, never in a scratch of
152+ // the workspace: with the capacity there it returns that storage.
153+ w := m.Current()
154+ big := make([]Placement, 0, 2*len(buf)+8)
155+ res := w.layoutInto(big)
156+ if len(res) == 0 || &res[0] != &big[:1][0] {
157+ t.Fatalf("%s: Workspace.layoutInto did not build in the caller's buffer", name)
158+ }
159+ if res = m.layoutInto(big); len(res) == 0 || &res[0] != &big[:1][0] {
160+ t.Fatalf("%s: Monitor.layoutInto did not build in the caller's buffer", name)
161+ }
162+ // A result kept across the calls that use the workspace scratch
163+ // (zoom tiles, preview tiles, columns, rows, a second layout) is
164+ // unchanged.
165+ mine := w.layoutInto(make([]Placement, 0, len(res)))
166+ mineKept := slices.Clone(mine)
167+ w.overviewZoom()
168+ w.peekStep()
169+ w.previewTiles()
170+ w.columnRects()
171+ _ = w.Layout()
172+ _ = w.layoutInto(nil)
173+ _ = m.Layout()
174+ if !slices.Equal(mine, mineKept) {
175+ t.Fatalf("%s: a kept layoutInto result changed across scratch-using calls\n got %+v\nwant %+v", name, mine, mineKept)
176+ }
148177 // Change the layout itself, then lay out again.
149178 if m.ov.open {
150179 m.OverviewMove(1, 0)
@@ -207,10 +236,54 @@ func cloneScene(s ports.Scene) ports.Scene {
207236 func TestPublishedScenesAreNeverMutated(t *testing.T) {
208237 type kept struct{ sent, copy ports.Scene }
209238 var all []kept
210- take := func(c *Core) {
239+ // fresh is every screen's layout as a fresh Layout() gives it, taken
240+ // before a step: the scene it publishes lists the same windows in the
241+ // same order, and, where nothing moves (no rect motion, no camera), the
242+ // same rects. A layout scratch shared with the real layouts the overview
243+ // reads would show here.
244+ type expect struct {
245+ out string
246+ layout []Placement
247+ still bool
248+ }
249+ fresh := func(c *Core) []expect {
250+ var e []expect
251+ for _, sc := range c.screens {
252+ if sc.name() != "" {
253+ e = append(e, expect{sc.name(), sc.mon.Layout(), len(sc.rects) == 0 && !sc.mon.Current().motion.on})
254+ }
255+ }
256+ return e
257+ }
258+ check := func(set []ports.Scene, want []expect, frame int) {
259+ t.Helper()
260+ if len(set) != len(want) {
261+ t.Fatalf("frame %d: %d scenes, %d screens", frame, len(set), len(want))
262+ }
263+ for i, s := range set {
264+ e := want[i]
265+ if s.Output != e.out || len(s.Windows) != len(e.layout) {
266+ t.Fatalf("frame %d: scene %s has %d windows, layout %d", frame, s.Output, len(s.Windows), len(e.layout))
267+ }
268+ for k, p := range e.layout {
269+ w := s.Windows[k]
270+ if w.ID != p.ID || w.Hidden != p.Hidden || (e.still && w.Rect != p.Rect) {
271+ t.Fatalf("frame %d: scene %s window %d is %+v, the layout has %+v", frame, s.Output, k, w, p)
272+ }
273+ }
274+ }
275+ }
276+ frame := 0
277+ step := func(c *Core) {
211278 t.Helper()
279+ want := fresh(c)
280+ if err := c.step(t.Context(), nil); err != nil {
281+ t.Fatal(err)
282+ }
212283 select {
213284 case set := <-c.ch.Scenes:
285+ check(set, want, frame)
286+ frame++
214287 for _, s := range set {
215288 all = append(all, kept{s, cloneScene(s)})
216289 }
@@ -231,10 +304,7 @@ func TestPublishedScenesAreNeverMutated(t *testing.T) {
231304 startRectMotions(c, sc, t0)
232305 }
233306 for range 12 {
234- if err := c.step(t.Context(), nil); err != nil {
235- t.Fatal(err)
236- }
237- take(c)
307+ step(c)
238308 }
239309
240310 // The overview opening: card motions, the real layouts, separators.
@@ -244,13 +314,24 @@ func TestPublishedScenesAreNeverMutated(t *testing.T) {
244314 o.cur().mon.ToggleOverview()
245315 o.transition(shots, now)
246316 for range 12 {
247- if err := o.step(t.Context(), nil); err != nil {
248- t.Fatal(err)
317+ step(o)
318+ }
319+ // The overview settled (no card motion): its scenes list the previews
320+ // exactly as the monitor lays them out, frame after frame.
321+ o.cur().stopRects()
322+ still := 0
323+ for range 3 {
324+ if w := fresh(o); !w[0].still {
325+ t.Fatal("setup: the overview still moves")
249326 }
250- take(o)
327+ step(o)
328+ still++
329+ }
330+ if !o.cur().mon.ov.open || still == 0 {
331+ t.Fatal("setup: overview closed")
251332 }
252333
253- if len(all) < 36 {
334+ if len(all) < 39 {
254335 t.Fatalf("only %d scenes", len(all))
255336 }
256337 changed := 0
@@ -268,3 +349,29 @@ func TestPublishedScenesAreNeverMutated(t *testing.T) {
268349 }
269350 }
270351 }
352+
353+// TestSettledLayoutEmptyFallsBackToLive: an empty layout is nil (as a fresh
354+// Layout() was), so a screen with nothing published yet measures the live
355+// layout: a window added after a publish of an empty monitor is listed by
356+// shownLayout without another publish.
357+func TestSettledLayoutEmptyFallsBackToLive(t *testing.T) {
358+ c := publishRig(t, 1)
359+ sc := c.cur()
360+ for _, p := range slices.Clone(sc.settledLayout) {
361+ sc.mon.RemoveWindow(p.ID)
362+ }
363+ if err := c.publish(t.Context()); err != nil {
364+ t.Fatal(err)
365+ }
366+ if sc.settledLayout != nil || sc.shown != nil {
367+ t.Fatalf("empty layout kept: settled %v shown %v (cap %d)", sc.settledLayout, sc.shown, cap(sc.layoutBuf))
368+ }
369+ if got := sc.shownLayout(); len(got) != 0 {
370+ t.Fatalf("empty monitor lists %+v", got)
371+ }
372+ sc.mon.AddWindow(77)
373+ got := sc.shownLayout()
374+ if len(got) != 1 || got[0].ID != 77 || got[0].Hidden {
375+ t.Fatalf("the window added after the publish is not listed: %+v", got)
376+ }
377+}
@@ -1,6 +1,7 @@
1 package core1 package core
2 2
3 import (3 import (
4+ "maps"
4 "reflect"5 "reflect"
5 "slices"6 "slices"
6 "testing"7 "testing"
@@ -49,9 +50,6 @@ func layoutVariants(t *testing.T) map[string]*Monitor {
49 50
50 m = base(OverflowScroll, 4)51 m = base(OverflowScroll, 4)
51 m.Current().Apply(ActionToggleWindowStash)52 m.Current().Apply(ActionToggleWindowStash)
52- m.Current().Apply(ActionToggleWindowStash)
53- m.Focus(2)
54- m.Current().Apply(ActionToggleWindowStash)
55 v["stash"] = m53 v["stash"] = m
56 54
57 m = base(OverflowScroll, 4)55 m = base(OverflowScroll, 4)
@@ -76,6 +74,21 @@ func layoutVariants(t *testing.T) map[string]*Monitor {
76 m.Apply(ActionFocusWorkspaceUp)74 m.Apply(ActionFocusWorkspaceUp)
77 m.switchOff = 0.475 m.switchOff = 0.4
78 v["slide"] = m76 v["slide"] = m
77+
78+ // The variants must hold what they are named for, on the workspace on
79+ // screen (the one whose scratch the tests exercise).
80+ for name, has := range map[string]func(*Workspace) bool{
81+ "stash": func(w *Workspace) bool { return len(w.Stash) > 0 && len(w.Columns) > 0 },
82+ "floats": func(w *Workspace) bool { return len(w.Floats) == 2 && len(w.Columns) > 0 },
83+ "shares": func(w *Workspace) bool { return validShares(w.Columns[0]) },
84+ "fixed-expanded": func(w *Workspace) bool { return w.Columns[1].Expanded },
85+ "fullscreen": func(w *Workspace) bool { return w.fullscreen == 2 },
86+ "scroll": func(w *Workspace) bool { return len(w.Columns) == 5 },
87+ } {
88+ if w := v[name].Current(); len(w.Columns) == 0 || !has(w) {
89+ t.Fatalf("variant %s: the workspace on screen lacks its feature: %+v", name, w.Columns)
90+ }
91+ }
79 return v92 return v
80 }93 }
81 94
@@ -85,7 +98,7 @@ func layoutVariants(t *testing.T) map[string]*Monitor {
85 func TestLayoutIntoMatchesLayout(t *testing.T) {98 func TestLayoutIntoMatchesLayout(t *testing.T) {
86 variants := layoutVariants(t)99 variants := layoutVariants(t)
87 var buf []Placement100 var buf []Placement
88- for _, name := range slices.Sorted(mapsKeys(variants)) {101+ for _, name := range slices.Sorted(maps.Keys(variants)) {
89 m := variants[name]102 m := variants[name]
90 want := m.Layout()103 want := m.Layout()
91 if len(want) == 0 {104 if len(want) == 0 {
@@ -113,16 +126,6 @@ func TestLayoutIntoMatchesLayout(t *testing.T) {
113 }126 }
114 }127 }
115 128
116-func mapsKeys[K comparable, V any](m map[K]V) func(func(K) bool) {
117- return func(yield func(K) bool) {
118- for k := range m {
119- if !yield(k) {
120- return
121- }
122- }
123- }
124-}
125-
126 // TestLayoutResultsDoNotAlias: Layout() returns a slice the caller owns. A129 // TestLayoutResultsDoNotAlias: Layout() returns a slice the caller owns. A
127 // retained result stays unchanged after further layouts of the same monitor130 // retained result stays unchanged after further layouts of the same monitor
128 // or workspace (nested or later, reused-buffer calls included), and two131 // or workspace (nested or later, reused-buffer calls included), and two
@@ -145,6 +148,32 @@ func TestLayoutResultsDoNotAlias(t *testing.T) {
145 m.Current().previewTiles()148 m.Current().previewTiles()
146 m.Current().overviewZoom()149 m.Current().overviewZoom()
147 }150 }
151+ // layoutInto builds in the caller's buffer, never in a scratch of
152+ // the workspace: with the capacity there it returns that storage.
153+ w := m.Current()
154+ big := make([]Placement, 0, 2*len(buf)+8)
155+ res := w.layoutInto(big)
156+ if len(res) == 0 || &res[0] != &big[:1][0] {
157+ t.Fatalf("%s: Workspace.layoutInto did not build in the caller's buffer", name)
158+ }
159+ if res = m.layoutInto(big); len(res) == 0 || &res[0] != &big[:1][0] {
160+ t.Fatalf("%s: Monitor.layoutInto did not build in the caller's buffer", name)
161+ }
162+ // A result kept across the calls that use the workspace scratch
163+ // (zoom tiles, preview tiles, columns, rows, a second layout) is
164+ // unchanged.
165+ mine := w.layoutInto(make([]Placement, 0, len(res)))
166+ mineKept := slices.Clone(mine)
167+ w.overviewZoom()
168+ w.peekStep()
169+ w.previewTiles()
170+ w.columnRects()
171+ _ = w.Layout()
172+ _ = w.layoutInto(nil)
173+ _ = m.Layout()
174+ if !slices.Equal(mine, mineKept) {
175+ t.Fatalf("%s: a kept layoutInto result changed across scratch-using calls\n got %+v\nwant %+v", name, mine, mineKept)
176+ }
148 // Change the layout itself, then lay out again.177 // Change the layout itself, then lay out again.
149 if m.ov.open {178 if m.ov.open {
150 m.OverviewMove(1, 0)179 m.OverviewMove(1, 0)
@@ -207,10 +236,54 @@ func cloneScene(s ports.Scene) ports.Scene {
207 func TestPublishedScenesAreNeverMutated(t *testing.T) {236 func TestPublishedScenesAreNeverMutated(t *testing.T) {
208 type kept struct{ sent, copy ports.Scene }237 type kept struct{ sent, copy ports.Scene }
209 var all []kept238 var all []kept
210- take := func(c *Core) {239+ // fresh is every screen's layout as a fresh Layout() gives it, taken
240+ // before a step: the scene it publishes lists the same windows in the
241+ // same order, and, where nothing moves (no rect motion, no camera), the
242+ // same rects. A layout scratch shared with the real layouts the overview
243+ // reads would show here.
244+ type expect struct {
245+ out string
246+ layout []Placement
247+ still bool
248+ }
249+ fresh := func(c *Core) []expect {
250+ var e []expect
251+ for _, sc := range c.screens {
252+ if sc.name() != "" {
253+ e = append(e, expect{sc.name(), sc.mon.Layout(), len(sc.rects) == 0 && !sc.mon.Current().motion.on})
254+ }
255+ }
256+ return e
257+ }
258+ check := func(set []ports.Scene, want []expect, frame int) {
259+ t.Helper()
260+ if len(set) != len(want) {
261+ t.Fatalf("frame %d: %d scenes, %d screens", frame, len(set), len(want))
262+ }
263+ for i, s := range set {
264+ e := want[i]
265+ if s.Output != e.out || len(s.Windows) != len(e.layout) {
266+ t.Fatalf("frame %d: scene %s has %d windows, layout %d", frame, s.Output, len(s.Windows), len(e.layout))
267+ }
268+ for k, p := range e.layout {
269+ w := s.Windows[k]
270+ if w.ID != p.ID || w.Hidden != p.Hidden || (e.still && w.Rect != p.Rect) {
271+ t.Fatalf("frame %d: scene %s window %d is %+v, the layout has %+v", frame, s.Output, k, w, p)
272+ }
273+ }
274+ }
275+ }
276+ frame := 0
277+ step := func(c *Core) {
211 t.Helper()278 t.Helper()
279+ want := fresh(c)
280+ if err := c.step(t.Context(), nil); err != nil {
281+ t.Fatal(err)
282+ }
212 select {283 select {
213 case set := <-c.ch.Scenes:284 case set := <-c.ch.Scenes:
285+ check(set, want, frame)
286+ frame++
214 for _, s := range set {287 for _, s := range set {
215 all = append(all, kept{s, cloneScene(s)})288 all = append(all, kept{s, cloneScene(s)})
216 }289 }
@@ -231,10 +304,7 @@ func TestPublishedScenesAreNeverMutated(t *testing.T) {
231 startRectMotions(c, sc, t0)304 startRectMotions(c, sc, t0)
232 }305 }
233 for range 12 {306 for range 12 {
234- if err := c.step(t.Context(), nil); err != nil {307+ step(c)
235- t.Fatal(err)
236- }
237- take(c)
238 }308 }
239 309
240 // The overview opening: card motions, the real layouts, separators.310 // The overview opening: card motions, the real layouts, separators.
@@ -244,13 +314,24 @@ func TestPublishedScenesAreNeverMutated(t *testing.T) {
244 o.cur().mon.ToggleOverview()314 o.cur().mon.ToggleOverview()
245 o.transition(shots, now)315 o.transition(shots, now)
246 for range 12 {316 for range 12 {
247- if err := o.step(t.Context(), nil); err != nil {317+ step(o)
248- t.Fatal(err)318+ }
319+ // The overview settled (no card motion): its scenes list the previews
320+ // exactly as the monitor lays them out, frame after frame.
321+ o.cur().stopRects()
322+ still := 0
323+ for range 3 {
324+ if w := fresh(o); !w[0].still {
325+ t.Fatal("setup: the overview still moves")
249 }326 }
250- take(o)327+ step(o)
328+ still++
329+ }
330+ if !o.cur().mon.ov.open || still == 0 {
331+ t.Fatal("setup: overview closed")
251 }332 }
252 333
253- if len(all) < 36 {334+ if len(all) < 39 {
254 t.Fatalf("only %d scenes", len(all))335 t.Fatalf("only %d scenes", len(all))
255 }336 }
256 changed := 0337 changed := 0
@@ -268,3 +349,29 @@ func TestPublishedScenesAreNeverMutated(t *testing.T) {
268 }349 }
269 }350 }
270 }351 }
352+
353+// TestSettledLayoutEmptyFallsBackToLive: an empty layout is nil (as a fresh
354+// Layout() was), so a screen with nothing published yet measures the live
355+// layout: a window added after a publish of an empty monitor is listed by
356+// shownLayout without another publish.
357+func TestSettledLayoutEmptyFallsBackToLive(t *testing.T) {
358+ c := publishRig(t, 1)
359+ sc := c.cur()
360+ for _, p := range slices.Clone(sc.settledLayout) {
361+ sc.mon.RemoveWindow(p.ID)
362+ }
363+ if err := c.publish(t.Context()); err != nil {
364+ t.Fatal(err)
365+ }
366+ if sc.settledLayout != nil || sc.shown != nil {
367+ t.Fatalf("empty layout kept: settled %v shown %v (cap %d)", sc.settledLayout, sc.shown, cap(sc.layoutBuf))
368+ }
369+ if got := sc.shownLayout(); len(got) != 0 {
370+ t.Fatalf("empty monitor lists %+v", got)
371+ }
372+ sc.mon.AddWindow(77)
373+ got := sc.shownLayout()
374+ if len(got) != 1 || got[0].ID != 77 || got[0].Hidden {
375+ t.Fatalf("the window added after the publish is not listed: %+v", got)
376+ }
377+}

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 26

Tags 10

↑↓ moveenter openesc close