detached 07742d2

fix(layout): harden pane tree operations

commit
07742d2bd01923696c65c4944950f4083b04b8f7
author
brice · · 2026-07-04 07:29 UTC
committer
brice · · 2026-07-04 07:41 UTC
parent
af961c3
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
3 files · +87 -32
Split
UnifiedSplit
M internal/usecase/layout/layout_test.go +41 -0
@@ -73,6 +73,20 @@ func TestSolveStackFitAndOverflow(t *testing.T) {
7373 require.False(t, ok)
7474 }
7575
76+func TestStackNewEmptyTreeReturnsError(t *testing.T) {
77+ t.Parallel()
78+ tr := &Tree{}
79+ require.ErrorIs(t, tr.StackNew("missing", "new", domain.Rect{Width: 20, Height: 2}), ErrNotFound)
80+ require.Nil(t, tr.Root)
81+ require.Empty(t, tr.Focus)
82+}
83+
84+func TestNilTreeCloneIsSafe(t *testing.T) {
85+ t.Parallel()
86+ var tr *Tree
87+ require.Nil(t, tr.clone())
88+}
89+
7690 func TestTooSmallRefusalsDoNotMutate(t *testing.T) {
7791 t.Parallel()
7892 tests := []struct {
@@ -218,6 +232,17 @@ func TestCloseRefocusPrefersSibling(t *testing.T) {
218232 require.Equal(t, PaneID("b"), tr.Focus)
219233 }
220234
235+func TestCloseNotFoundDoesNotMutate(t *testing.T) {
236+ t.Parallel()
237+ tr := &Tree{Focus: "b", Root: &Node{Kind: Split, Dir: Horizontal, Children: []*Node{
238+ NewLeaf("a"),
239+ {Kind: Stack, Children: []*Node{NewLeaf("b"), NewLeaf("c")}, Expanded: "b"},
240+ }}}
241+ before := tr.clone()
242+ require.ErrorIs(t, tr.Close("missing"), ErrNotFound)
243+ require.Equal(t, before, tr)
244+}
245+
221246 func TestSolveRejectsStackWithMissingExpanded(t *testing.T) {
222247 t.Parallel()
223248 _, ok := Solve(&Node{Kind: Stack, Children: []*Node{NewLeaf("a"), NewLeaf("b")}, Expanded: "missing"}, domain.Rect{Width: 20, Height: 4})
@@ -342,6 +367,22 @@ func TestFocusDirEnteringStackExpandsMember(t *testing.T) {
342367 }
343368 }
344369
370+func TestFocusDirStackLocalSkipsNonLeafChildren(t *testing.T) {
371+ t.Parallel()
372+ tr := &Tree{
373+ Root: &Node{Kind: Stack, Children: []*Node{
374+ NewLeaf("a"),
375+ {Kind: Split, Dir: Horizontal, Children: []*Node{NewLeaf("nested")}},
376+ NewLeaf("b"),
377+ }, Expanded: "a"},
378+ Focus: "a",
379+ }
380+
381+ require.NoError(t, tr.FocusDir(Down, domain.Rect{Width: 80, Height: 6}))
382+ require.Equal(t, PaneID("b"), tr.Focus)
383+ require.Equal(t, PaneID("b"), tr.Root.Expanded)
384+}
385+
345386 func contents(ps []Placement) map[PaneID]domain.Rect {
346387 out := make(map[PaneID]domain.Rect, len(ps))
347388 for _, p := range ps {
@@ -73,6 +73,20 @@ func TestSolveStackFitAndOverflow(t *testing.T) {
73 require.False(t, ok)73 require.False(t, ok)
74 }74 }
75 75
76+func TestStackNewEmptyTreeReturnsError(t *testing.T) {
77+ t.Parallel()
78+ tr := &Tree{}
79+ require.ErrorIs(t, tr.StackNew("missing", "new", domain.Rect{Width: 20, Height: 2}), ErrNotFound)
80+ require.Nil(t, tr.Root)
81+ require.Empty(t, tr.Focus)
82+}
83+
84+func TestNilTreeCloneIsSafe(t *testing.T) {
85+ t.Parallel()
86+ var tr *Tree
87+ require.Nil(t, tr.clone())
88+}
89+
76 func TestTooSmallRefusalsDoNotMutate(t *testing.T) {90 func TestTooSmallRefusalsDoNotMutate(t *testing.T) {
77 t.Parallel()91 t.Parallel()
78 tests := []struct {92 tests := []struct {
@@ -218,6 +232,17 @@ func TestCloseRefocusPrefersSibling(t *testing.T) {
218 require.Equal(t, PaneID("b"), tr.Focus)232 require.Equal(t, PaneID("b"), tr.Focus)
219 }233 }
220 234
235+func TestCloseNotFoundDoesNotMutate(t *testing.T) {
236+ t.Parallel()
237+ tr := &Tree{Focus: "b", Root: &Node{Kind: Split, Dir: Horizontal, Children: []*Node{
238+ NewLeaf("a"),
239+ {Kind: Stack, Children: []*Node{NewLeaf("b"), NewLeaf("c")}, Expanded: "b"},
240+ }}}
241+ before := tr.clone()
242+ require.ErrorIs(t, tr.Close("missing"), ErrNotFound)
243+ require.Equal(t, before, tr)
244+}
245+
221 func TestSolveRejectsStackWithMissingExpanded(t *testing.T) {246 func TestSolveRejectsStackWithMissingExpanded(t *testing.T) {
222 t.Parallel()247 t.Parallel()
223 _, ok := Solve(&Node{Kind: Stack, Children: []*Node{NewLeaf("a"), NewLeaf("b")}, Expanded: "missing"}, domain.Rect{Width: 20, Height: 4})248 _, ok := Solve(&Node{Kind: Stack, Children: []*Node{NewLeaf("a"), NewLeaf("b")}, Expanded: "missing"}, domain.Rect{Width: 20, Height: 4})
@@ -342,6 +367,22 @@ func TestFocusDirEnteringStackExpandsMember(t *testing.T) {
342 }367 }
343 }368 }
344 369
370+func TestFocusDirStackLocalSkipsNonLeafChildren(t *testing.T) {
371+ t.Parallel()
372+ tr := &Tree{
373+ Root: &Node{Kind: Stack, Children: []*Node{
374+ NewLeaf("a"),
375+ {Kind: Split, Dir: Horizontal, Children: []*Node{NewLeaf("nested")}},
376+ NewLeaf("b"),
377+ }, Expanded: "a"},
378+ Focus: "a",
379+ }
380+
381+ require.NoError(t, tr.FocusDir(Down, domain.Rect{Width: 80, Height: 6}))
382+ require.Equal(t, PaneID("b"), tr.Focus)
383+ require.Equal(t, PaneID("b"), tr.Root.Expanded)
384+}
385+
345 func contents(ps []Placement) map[PaneID]domain.Rect {386 func contents(ps []Placement) map[PaneID]domain.Rect {
346 out := make(map[PaneID]domain.Rect, len(ps))387 out := make(map[PaneID]domain.Rect, len(ps))
347 for _, p := range ps {388 for _, p := range ps {
M internal/usecase/layout/ops.go +43 -32
@@ -68,7 +68,7 @@ func insertSplit(n *Node, target PaneID, axis SplitDir, after bool, newID PaneID
6868 // StackNew puts newID in target's stack, creating one if target is not stacked.
6969 func (t *Tree) StackNew(target, newID PaneID, area domain.Rect) error {
7070 candidate := t.clone()
71- if !stackNew(candidate.Root, target, newID) {
71+ if candidate == nil || !stackNew(candidate.Root, target, newID) {
7272 return ErrNotFound
7373 }
7474 candidate.Focus = newID
@@ -80,6 +80,9 @@ func (t *Tree) StackNew(target, newID PaneID, area domain.Rect) error {
8080 }
8181
8282 func stackNew(n *Node, target, newID PaneID) bool {
83+ if n == nil {
84+ return false
85+ }
8386 if n.Kind == Stack {
8487 for _, child := range n.Children {
8588 if child.Kind == Leaf && child.Leaf == target {
@@ -184,33 +187,36 @@ func splitDirectLeavesContain(n *Node, target PaneID) bool {
184187
185188 // Close removes target and dissolves single-child containers.
186189 func (t *Tree) Close(target PaneID) error {
187- if t.Root == nil || !containsLeaf(t.Root, target) {
190+ candidate := t.clone()
191+ if candidate == nil || candidate.Root == nil || !containsLeaf(candidate.Root, target) {
188192 return ErrNotFound
189193 }
190- if t.Root.Kind == Leaf && t.Root.Leaf == target {
191- t.Root = nil
192- t.Focus = ""
194+ if candidate.Root.Kind == Leaf && candidate.Root.Leaf == target {
195+ candidate.Root = nil
196+ candidate.Focus = ""
197+ *t = *candidate
193198 return nil
194199 }
195- refocus := closeRefocusCandidate(t.Root, target)
196- root, removed := closeNode(t.Root, target)
200+ refocus := closeRefocusCandidate(candidate.Root, target)
201+ root, removed := closeNode(candidate.Root, target)
197202 if !removed {
198203 return ErrNotFound
199204 }
200- t.Root = root
201- if t.Focus == target {
202- if refocus != "" && containsLeaf(t.Root, refocus) {
203- t.Focus = refocus
205+ candidate.Root = root
206+ if candidate.Focus == target {
207+ if refocus != "" && containsLeaf(candidate.Root, refocus) {
208+ candidate.Focus = refocus
204209 } else {
205- ids := leafIDs(t.Root)
210+ ids := leafIDs(candidate.Root)
206211 if len(ids) > 0 {
207- t.Focus = ids[0]
212+ candidate.Focus = ids[0]
208213 } else {
209- t.Focus = ""
214+ candidate.Focus = ""
210215 }
211216 }
212- setExpanded(t.Root, t.Focus)
217+ setExpanded(candidate.Root, candidate.Focus)
213218 }
219+ *t = *candidate
214220 return nil
215221 }
216222
@@ -412,24 +418,11 @@ func (t *Tree) FocusDir(dir Direction, area domain.Rect) error {
412418
413419 func focusStackLocal(n *Node, id PaneID, dir Direction, focus *PaneID) bool {
414420 if n.Kind == Stack {
415- idx := -1
416- for i, child := range n.Children {
417- if child.Kind == Leaf && child.Leaf == id {
418- idx = i
419- break
420- }
421- }
421+ idx := stackDirectLeafIndex(n, id)
422422 if idx >= 0 {
423- next := idx
424- if dir == Up && idx > 0 {
425- next = idx - 1
426- }
427- if dir == Down && idx < len(n.Children)-1 {
428- next = idx + 1
429- }
430- if next != idx {
431- n.Expanded = n.Children[next].Leaf
432- *focus = n.Children[next].Leaf
423+ if next := adjacentStackLeaf(n, idx, dir); next != "" {
424+ n.Expanded = next
425+ *focus = next
433426 return true
434427 }
435428 return false
@@ -443,6 +436,24 @@ func focusStackLocal(n *Node, id PaneID, dir Direction, focus *PaneID) bool {
443436 return false
444437 }
445438
439+func adjacentStackLeaf(n *Node, idx int, dir Direction) PaneID {
440+ step := 0
441+ switch dir {
442+ case Up:
443+ step = -1
444+ case Down:
445+ step = 1
446+ default:
447+ return ""
448+ }
449+ for i := idx + step; i >= 0 && i < len(n.Children); i += step {
450+ if n.Children[i].Kind == Leaf {
451+ return n.Children[i].Leaf
452+ }
453+ }
454+ return ""
455+}
456+
446457 func focusRect(p Placement) domain.Rect {
447458 if p.TitleBar.Height == 0 {
448459 return p.Content
@@ -68,7 +68,7 @@ func insertSplit(n *Node, target PaneID, axis SplitDir, after bool, newID PaneID
68 // StackNew puts newID in target's stack, creating one if target is not stacked.68 // StackNew puts newID in target's stack, creating one if target is not stacked.
69 func (t *Tree) StackNew(target, newID PaneID, area domain.Rect) error {69 func (t *Tree) StackNew(target, newID PaneID, area domain.Rect) error {
70 candidate := t.clone()70 candidate := t.clone()
71- if !stackNew(candidate.Root, target, newID) {71+ if candidate == nil || !stackNew(candidate.Root, target, newID) {
72 return ErrNotFound72 return ErrNotFound
73 }73 }
74 candidate.Focus = newID74 candidate.Focus = newID
@@ -80,6 +80,9 @@ func (t *Tree) StackNew(target, newID PaneID, area domain.Rect) error {
80 }80 }
81 81
82 func stackNew(n *Node, target, newID PaneID) bool {82 func stackNew(n *Node, target, newID PaneID) bool {
83+ if n == nil {
84+ return false
85+ }
83 if n.Kind == Stack {86 if n.Kind == Stack {
84 for _, child := range n.Children {87 for _, child := range n.Children {
85 if child.Kind == Leaf && child.Leaf == target {88 if child.Kind == Leaf && child.Leaf == target {
@@ -184,33 +187,36 @@ func splitDirectLeavesContain(n *Node, target PaneID) bool {
184 187
185 // Close removes target and dissolves single-child containers.188 // Close removes target and dissolves single-child containers.
186 func (t *Tree) Close(target PaneID) error {189 func (t *Tree) Close(target PaneID) error {
187- if t.Root == nil || !containsLeaf(t.Root, target) {190+ candidate := t.clone()
191+ if candidate == nil || candidate.Root == nil || !containsLeaf(candidate.Root, target) {
188 return ErrNotFound192 return ErrNotFound
189 }193 }
190- if t.Root.Kind == Leaf && t.Root.Leaf == target {194+ if candidate.Root.Kind == Leaf && candidate.Root.Leaf == target {
191- t.Root = nil195+ candidate.Root = nil
192- t.Focus = ""196+ candidate.Focus = ""
197+ *t = *candidate
193 return nil198 return nil
194 }199 }
195- refocus := closeRefocusCandidate(t.Root, target)200+ refocus := closeRefocusCandidate(candidate.Root, target)
196- root, removed := closeNode(t.Root, target)201+ root, removed := closeNode(candidate.Root, target)
197 if !removed {202 if !removed {
198 return ErrNotFound203 return ErrNotFound
199 }204 }
200- t.Root = root205+ candidate.Root = root
201- if t.Focus == target {206+ if candidate.Focus == target {
202- if refocus != "" && containsLeaf(t.Root, refocus) {207+ if refocus != "" && containsLeaf(candidate.Root, refocus) {
203- t.Focus = refocus208+ candidate.Focus = refocus
204 } else {209 } else {
205- ids := leafIDs(t.Root)210+ ids := leafIDs(candidate.Root)
206 if len(ids) > 0 {211 if len(ids) > 0 {
207- t.Focus = ids[0]212+ candidate.Focus = ids[0]
208 } else {213 } else {
209- t.Focus = ""214+ candidate.Focus = ""
210 }215 }
211 }216 }
212- setExpanded(t.Root, t.Focus)217+ setExpanded(candidate.Root, candidate.Focus)
213 }218 }
219+ *t = *candidate
214 return nil220 return nil
215 }221 }
216 222
@@ -412,24 +418,11 @@ func (t *Tree) FocusDir(dir Direction, area domain.Rect) error {
412 418
413 func focusStackLocal(n *Node, id PaneID, dir Direction, focus *PaneID) bool {419 func focusStackLocal(n *Node, id PaneID, dir Direction, focus *PaneID) bool {
414 if n.Kind == Stack {420 if n.Kind == Stack {
415- idx := -1421+ idx := stackDirectLeafIndex(n, id)
416- for i, child := range n.Children {
417- if child.Kind == Leaf && child.Leaf == id {
418- idx = i
419- break
420- }
421- }
422 if idx >= 0 {422 if idx >= 0 {
423- next := idx423+ if next := adjacentStackLeaf(n, idx, dir); next != "" {
424- if dir == Up && idx > 0 {424+ n.Expanded = next
425- next = idx - 1425+ *focus = next
426- }
427- if dir == Down && idx < len(n.Children)-1 {
428- next = idx + 1
429- }
430- if next != idx {
431- n.Expanded = n.Children[next].Leaf
432- *focus = n.Children[next].Leaf
433 return true426 return true
434 }427 }
435 return false428 return false
@@ -443,6 +436,24 @@ func focusStackLocal(n *Node, id PaneID, dir Direction, focus *PaneID) bool {
443 return false436 return false
444 }437 }
445 438
439+func adjacentStackLeaf(n *Node, idx int, dir Direction) PaneID {
440+ step := 0
441+ switch dir {
442+ case Up:
443+ step = -1
444+ case Down:
445+ step = 1
446+ default:
447+ return ""
448+ }
449+ for i := idx + step; i >= 0 && i < len(n.Children); i += step {
450+ if n.Children[i].Kind == Leaf {
451+ return n.Children[i].Leaf
452+ }
453+ }
454+ return ""
455+}
456+
446 func focusRect(p Placement) domain.Rect {457 func focusRect(p Placement) domain.Rect {
447 if p.TitleBar.Height == 0 {458 if p.TitleBar.Height == 0 {
448 return p.Content459 return p.Content
M internal/usecase/layout/tree.go +3 -0
@@ -89,5 +89,8 @@ func (n *Node) clone() *Node {
8989 }
9090
9191 func (t *Tree) clone() *Tree {
92+ if t == nil {
93+ return nil
94+ }
9295 return &Tree{Root: t.Root.clone(), Focus: t.Focus}
9396 }
@@ -89,5 +89,8 @@ func (n *Node) clone() *Node {
89 }89 }
90 90
91 func (t *Tree) clone() *Tree {91 func (t *Tree) clone() *Tree {
92+ if t == nil {
93+ return nil
94+ }
92 return &Tree{Root: t.Root.clone(), Focus: t.Focus}95 return &Tree{Root: t.Root.clone(), Focus: t.Focus}
93 }96 }

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