refactor(daemon): prepare directional focus overflow
- commit
- 15f2a25f67486063a328425d0ed4ab791ebb6f2b
- author
- brice · · 2026-07-25 05:09 UTC
- parent
- 9d0a15c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +61 -30
M internal/usecase/daemon/control.go +4 -12
| @@ -265,7 +265,8 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||
| 265 | 265 | } | |
| 266 | 266 | return nil | |
| 267 | 267 | case daemonActionFocusPane: | |
| 268 | - | return a.d.focusDirAt(target.session, target.tab, target.pane, request.direction) | |
| 268 | + | _, err := a.d.focusDirAt(target.session, target.tab, target.pane, request.direction) | |
| 269 | + | return err | |
| 269 | 270 | case daemonActionNextTab: | |
| 270 | 271 | return a.switchRelative(target.session, 1) | |
| 271 | 272 | case daemonActionPreviousTab: | |
| @@ -349,17 +350,8 @@ func finishDaemonActionForClient(d *Daemon, request daemonActionRequest, ac *att | |||
| 349 | 350 | return | |
| 350 | 351 | } | |
| 351 | 352 | if request.kind == daemonActionFocusPane && request.target.tab != nil && request.target.pane != nil { | |
| 352 | - | tb := request.target.tab | |
| 353 | - | tb.mu.Lock() | |
| 354 | - | newFocus := tb.tree.Focus | |
| 355 | - | pl, hasPlacement := focusedPlacementLocked(tb) | |
| 356 | - | tb.mu.Unlock() | |
| 357 | - | if newFocus != request.target.pane.id { | |
| 358 | - | d.exitCopyMode(ac) | |
| 359 | - | if hasPlacement && pl.InStack { | |
| 360 | - | d.refreshPaneTitleOnFocus(request.target.session, newFocus) | |
| 361 | - | } | |
| 362 | - | } | |
| 353 | + | d.finishPaneFocusForClient(request.target.session, ac, request.target.tab, request.target.pane.id, producer) | |
| 354 | + | return | |
| 363 | 355 | } | |
| 364 | 356 | d.invalidateRender(request.target.session, ac, true, producer) | |
| 365 | 357 | } | |
| @@ -265,7 +265,8 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||||
| 265 | } | 265 | } | ||
| 266 | return nil | 266 | return nil | ||
| 267 | case daemonActionFocusPane: | 267 | case daemonActionFocusPane: | ||
| 268 | - | return a.d.focusDirAt(target.session, target.tab, target.pane, request.direction) | 268 | + | _, err := a.d.focusDirAt(target.session, target.tab, target.pane, request.direction) |
| 269 | + | return err | |||
| 269 | case daemonActionNextTab: | 270 | case daemonActionNextTab: | ||
| 270 | return a.switchRelative(target.session, 1) | 271 | return a.switchRelative(target.session, 1) | ||
| 271 | case daemonActionPreviousTab: | 272 | case daemonActionPreviousTab: | ||
| @@ -349,17 +350,8 @@ func finishDaemonActionForClient(d *Daemon, request daemonActionRequest, ac *att | |||||
| 349 | return | 350 | return | ||
| 350 | } | 351 | } | ||
| 351 | if request.kind == daemonActionFocusPane && request.target.tab != nil && request.target.pane != nil { | 352 | if request.kind == daemonActionFocusPane && request.target.tab != nil && request.target.pane != nil { | ||
| 352 | - | tb := request.target.tab | 353 | + | d.finishPaneFocusForClient(request.target.session, ac, request.target.tab, request.target.pane.id, producer) |
| 353 | - | tb.mu.Lock() | 354 | + | return |
| 354 | - | newFocus := tb.tree.Focus | |||
| 355 | - | pl, hasPlacement := focusedPlacementLocked(tb) | |||
| 356 | - | tb.mu.Unlock() | |||
| 357 | - | if newFocus != request.target.pane.id { | |||
| 358 | - | d.exitCopyMode(ac) | |||
| 359 | - | if hasPlacement && pl.InStack { | |||
| 360 | - | d.refreshPaneTitleOnFocus(request.target.session, newFocus) | |||
| 361 | - | } | |||
| 362 | - | } | |||
| 363 | } | 355 | } | ||
| 364 | d.invalidateRender(request.target.session, ac, true, producer) | 356 | d.invalidateRender(request.target.session, ac, true, producer) | ||
| 365 | } | 357 | } | ||
M internal/usecase/daemon/pane_actions.go +27 -18
| @@ -309,35 +309,44 @@ func (d *Daemon) focusDir(sess *session, ac *attachedClient, dir layout.Directio | |||
| 309 | 309 | if target.pane != nil { | |
| 310 | 310 | oldFocus = target.pane.id | |
| 311 | 311 | } | |
| 312 | - | err := d.focusDirAt(sess, target.tab, target.pane, dir) | |
| 312 | + | _, err := d.focusDirAt(sess, target.tab, target.pane, dir) | |
| 313 | 313 | if err == nil && ac != nil { | |
| 314 | - | target.tab.mu.Lock() | |
| 315 | - | newFocus := target.tab.tree.Focus | |
| 316 | - | pl, hasPlacement := focusedPlacementLocked(target.tab) | |
| 317 | - | target.tab.mu.Unlock() | |
| 318 | - | if newFocus != oldFocus { | |
| 319 | - | d.exitCopyMode(ac) | |
| 320 | - | if hasPlacement && pl.InStack { | |
| 321 | - | d.refreshPaneTitleOnFocus(sess, newFocus) | |
| 322 | - | } | |
| 323 | - | } | |
| 324 | - | d.invalidateRender(sess, ac, true, "pane_actions.go") | |
| 314 | + | d.finishPaneFocusForClient(sess, ac, target.tab, oldFocus, "pane_actions.go") | |
| 325 | 315 | } | |
| 326 | 316 | return err | |
| 327 | 317 | } | |
| 328 | 318 | ||
| 329 | - | func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Direction) error { | |
| 319 | + | // finishPaneFocusForClient applies the attachment lifecycle shared by every | |
| 320 | + | // successful directional focus move. Keep copy-mode exit before the optional | |
| 321 | + | // stack title refresh, and both before render invalidation. | |
| 322 | + | func (d *Daemon) finishPaneFocusForClient(sess *session, ac *attachedClient, tb *tab, oldFocus layout.PaneID, producer string) { | |
| 323 | + | tb.mu.Lock() | |
| 324 | + | newFocus := tb.tree.Focus | |
| 325 | + | pl, hasPlacement := focusedPlacementLocked(tb) | |
| 326 | + | tb.mu.Unlock() | |
| 327 | + | if newFocus != oldFocus { | |
| 328 | + | d.exitCopyMode(ac) | |
| 329 | + | if hasPlacement && pl.InStack { | |
| 330 | + | d.refreshPaneTitleOnFocus(sess, newFocus) | |
| 331 | + | } | |
| 332 | + | } | |
| 333 | + | d.invalidateRender(sess, ac, true, producer) | |
| 334 | + | } | |
| 335 | + | ||
| 336 | + | func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Direction) (domain.Rect, error) { | |
| 330 | 337 | if tb == nil || target == nil { | |
| 331 | - | return layout.ErrNotFound | |
| 338 | + | return domain.Rect{}, layout.ErrNotFound | |
| 332 | 339 | } | |
| 333 | 340 | tb.mu.Lock() | |
| 334 | 341 | if tb.tree == nil || tb.panes[target.id] != target || !layout.ContainsLeaf(tb.tree.Root, target.id) { | |
| 335 | 342 | tb.mu.Unlock() | |
| 336 | - | return layout.ErrNotFound | |
| 343 | + | return domain.Rect{}, layout.ErrNotFound | |
| 337 | 344 | } | |
| 338 | 345 | oldFocus := target.id | |
| 339 | 346 | tb.tree.Focus = oldFocus | |
| 340 | - | err := tb.tree.FocusDir(dir, domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows}) | |
| 347 | + | area := domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows} | |
| 348 | + | span, _ := tb.tree.FocusSpan(area) | |
| 349 | + | err := tb.tree.FocusDir(dir, area) | |
| 341 | 350 | newFocus := tb.tree.Focus | |
| 342 | 351 | if err == nil { | |
| 343 | 352 | d.applyLayoutLocked(tb) | |
| @@ -347,7 +356,7 @@ func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Dir | |||
| 347 | 356 | markSnapshotDirty(sess) | |
| 348 | 357 | } | |
| 349 | 358 | if errors.Is(err, layout.ErrNoPane) { | |
| 350 | - | return errNoNeighbor | |
| 359 | + | return span, errNoNeighbor | |
| 351 | 360 | } | |
| 352 | - | return err | |
| 361 | + | return span, err | |
| 353 | 362 | } | |
| @@ -309,35 +309,44 @@ func (d *Daemon) focusDir(sess *session, ac *attachedClient, dir layout.Directio | |||||
| 309 | if target.pane != nil { | 309 | if target.pane != nil { | ||
| 310 | oldFocus = target.pane.id | 310 | oldFocus = target.pane.id | ||
| 311 | } | 311 | } | ||
| 312 | - | err := d.focusDirAt(sess, target.tab, target.pane, dir) | 312 | + | _, err := d.focusDirAt(sess, target.tab, target.pane, dir) |
| 313 | if err == nil && ac != nil { | 313 | if err == nil && ac != nil { | ||
| 314 | - | target.tab.mu.Lock() | 314 | + | d.finishPaneFocusForClient(sess, ac, target.tab, oldFocus, "pane_actions.go") |
| 315 | - | newFocus := target.tab.tree.Focus | |||
| 316 | - | pl, hasPlacement := focusedPlacementLocked(target.tab) | |||
| 317 | - | target.tab.mu.Unlock() | |||
| 318 | - | if newFocus != oldFocus { | |||
| 319 | - | d.exitCopyMode(ac) | |||
| 320 | - | if hasPlacement && pl.InStack { | |||
| 321 | - | d.refreshPaneTitleOnFocus(sess, newFocus) | |||
| 322 | - | } | |||
| 323 | - | } | |||
| 324 | - | d.invalidateRender(sess, ac, true, "pane_actions.go") | |||
| 325 | } | 315 | } | ||
| 326 | return err | 316 | return err | ||
| 327 | } | 317 | } | ||
| 328 | 318 | ||||
| 329 | - | func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Direction) error { | 319 | + | // finishPaneFocusForClient applies the attachment lifecycle shared by every |
| 320 | + | // successful directional focus move. Keep copy-mode exit before the optional | |||
| 321 | + | // stack title refresh, and both before render invalidation. | |||
| 322 | + | func (d *Daemon) finishPaneFocusForClient(sess *session, ac *attachedClient, tb *tab, oldFocus layout.PaneID, producer string) { | |||
| 323 | + | tb.mu.Lock() | |||
| 324 | + | newFocus := tb.tree.Focus | |||
| 325 | + | pl, hasPlacement := focusedPlacementLocked(tb) | |||
| 326 | + | tb.mu.Unlock() | |||
| 327 | + | if newFocus != oldFocus { | |||
| 328 | + | d.exitCopyMode(ac) | |||
| 329 | + | if hasPlacement && pl.InStack { | |||
| 330 | + | d.refreshPaneTitleOnFocus(sess, newFocus) | |||
| 331 | + | } | |||
| 332 | + | } | |||
| 333 | + | d.invalidateRender(sess, ac, true, producer) | |||
| 334 | + | } | |||
| 335 | + | ||||
| 336 | + | func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Direction) (domain.Rect, error) { | |||
| 330 | if tb == nil || target == nil { | 337 | if tb == nil || target == nil { | ||
| 331 | - | return layout.ErrNotFound | 338 | + | return domain.Rect{}, layout.ErrNotFound |
| 332 | } | 339 | } | ||
| 333 | tb.mu.Lock() | 340 | tb.mu.Lock() | ||
| 334 | if tb.tree == nil || tb.panes[target.id] != target || !layout.ContainsLeaf(tb.tree.Root, target.id) { | 341 | if tb.tree == nil || tb.panes[target.id] != target || !layout.ContainsLeaf(tb.tree.Root, target.id) { | ||
| 335 | tb.mu.Unlock() | 342 | tb.mu.Unlock() | ||
| 336 | - | return layout.ErrNotFound | 343 | + | return domain.Rect{}, layout.ErrNotFound |
| 337 | } | 344 | } | ||
| 338 | oldFocus := target.id | 345 | oldFocus := target.id | ||
| 339 | tb.tree.Focus = oldFocus | 346 | tb.tree.Focus = oldFocus | ||
| 340 | - | err := tb.tree.FocusDir(dir, domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows}) | 347 | + | area := domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows} |
| 348 | + | span, _ := tb.tree.FocusSpan(area) | |||
| 349 | + | err := tb.tree.FocusDir(dir, area) | |||
| 341 | newFocus := tb.tree.Focus | 350 | newFocus := tb.tree.Focus | ||
| 342 | if err == nil { | 351 | if err == nil { | ||
| 343 | d.applyLayoutLocked(tb) | 352 | d.applyLayoutLocked(tb) | ||
| @@ -347,7 +356,7 @@ func (d *Daemon) focusDirAt(sess *session, tb *tab, target *pane, dir layout.Dir | |||||
| 347 | markSnapshotDirty(sess) | 356 | markSnapshotDirty(sess) | ||
| 348 | } | 357 | } | ||
| 349 | if errors.Is(err, layout.ErrNoPane) { | 358 | if errors.Is(err, layout.ErrNoPane) { | ||
| 350 | - | return errNoNeighbor | 359 | + | return span, errNoNeighbor |
| 351 | } | 360 | } | ||
| 352 | - | return err | 361 | + | return span, err |
| 353 | } | 362 | } | ||
M internal/usecase/daemon/pane_actions_test.go +30 -0
| @@ -152,6 +152,36 @@ func TestResizeReflowsAllPanes(t *testing.T) { | |||
| 152 | 152 | require.Equal(t, 29, tb.panes["pane-2"].screen.Frame.Width) | |
| 153 | 153 | } | |
| 154 | 154 | ||
| 155 | + | func TestFocusDirAtReturnsDepartingSpanAndMapsNoNeighbor(t *testing.T) { | |
| 156 | + | tests := []struct { | |
| 157 | + | name string | |
| 158 | + | direction layout.Direction | |
| 159 | + | wantErr error | |
| 160 | + | wantFocus layout.PaneID | |
| 161 | + | }{ | |
| 162 | + | {name: "moves right", direction: layout.Right, wantFocus: "pane-2"}, | |
| 163 | + | {name: "maps missing left neighbor", direction: layout.Left, wantErr: errNoNeighbor, wantFocus: "pane-1"}, | |
| 164 | + | } | |
| 165 | + | ||
| 166 | + | for _, tt := range tests { | |
| 167 | + | t.Run(tt.name, func(t *testing.T) { | |
| 168 | + | d, sess, _, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | |
| 169 | + | tb := sess.activeTab() | |
| 170 | + | target := tb.panes["pane-1"] | |
| 171 | + | tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2")}}, Focus: "pane-1"} | |
| 172 | + | tb.panes["pane-1"].rect = domain.Rect{Width: 20, Height: 10} | |
| 173 | + | tb.panes["pane-2"] = newPane("pane-2", nil, domain.Size{Cols: 20, Rows: 10}) | |
| 174 | + | tb.panes["pane-2"].rect = domain.Rect{X: 21, Width: 20, Height: 10} | |
| 175 | + | ||
| 176 | + | span, err := d.focusDirAt(sess, tb, target, tt.direction) | |
| 177 | + | ||
| 178 | + | require.ErrorIs(t, err, tt.wantErr) | |
| 179 | + | require.Equal(t, domain.Rect{Width: 20, Height: 10}, span) | |
| 180 | + | require.Equal(t, tt.wantFocus, tb.tree.Focus) | |
| 181 | + | }) | |
| 182 | + | } | |
| 183 | + | } | |
| 184 | + | ||
| 155 | 185 | func TestFocusDirMovesFocusAndExitsCopyMode(t *testing.T) { | |
| 156 | 186 | d, sess, oldPTY, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | |
| 157 | 187 | tb := sess.activeTab() | |
| @@ -152,6 +152,36 @@ func TestResizeReflowsAllPanes(t *testing.T) { | |||||
| 152 | require.Equal(t, 29, tb.panes["pane-2"].screen.Frame.Width) | 152 | require.Equal(t, 29, tb.panes["pane-2"].screen.Frame.Width) | ||
| 153 | } | 153 | } | ||
| 154 | 154 | ||||
| 155 | + | func TestFocusDirAtReturnsDepartingSpanAndMapsNoNeighbor(t *testing.T) { | |||
| 156 | + | tests := []struct { | |||
| 157 | + | name string | |||
| 158 | + | direction layout.Direction | |||
| 159 | + | wantErr error | |||
| 160 | + | wantFocus layout.PaneID | |||
| 161 | + | }{ | |||
| 162 | + | {name: "moves right", direction: layout.Right, wantFocus: "pane-2"}, | |||
| 163 | + | {name: "maps missing left neighbor", direction: layout.Left, wantErr: errNoNeighbor, wantFocus: "pane-1"}, | |||
| 164 | + | } | |||
| 165 | + | ||||
| 166 | + | for _, tt := range tests { | |||
| 167 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 168 | + | d, sess, _, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | |||
| 169 | + | tb := sess.activeTab() | |||
| 170 | + | target := tb.panes["pane-1"] | |||
| 171 | + | tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2")}}, Focus: "pane-1"} | |||
| 172 | + | tb.panes["pane-1"].rect = domain.Rect{Width: 20, Height: 10} | |||
| 173 | + | tb.panes["pane-2"] = newPane("pane-2", nil, domain.Size{Cols: 20, Rows: 10}) | |||
| 174 | + | tb.panes["pane-2"].rect = domain.Rect{X: 21, Width: 20, Height: 10} | |||
| 175 | + | ||||
| 176 | + | span, err := d.focusDirAt(sess, tb, target, tt.direction) | |||
| 177 | + | ||||
| 178 | + | require.ErrorIs(t, err, tt.wantErr) | |||
| 179 | + | require.Equal(t, domain.Rect{Width: 20, Height: 10}, span) | |||
| 180 | + | require.Equal(t, tt.wantFocus, tb.tree.Focus) | |||
| 181 | + | }) | |||
| 182 | + | } | |||
| 183 | + | } | |||
| 184 | + | ||||
| 155 | func TestFocusDirMovesFocusAndExitsCopyMode(t *testing.T) { | 185 | func TestFocusDirMovesFocusAndExitsCopyMode(t *testing.T) { | ||
| 156 | d, sess, oldPTY, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | 186 | d, sess, oldPTY, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | ||
| 157 | tb := sess.activeTab() | 187 | tb := sess.activeTab() | ||