fix(daemon): preserve floating title fallback
- commit
- 9f11203a36b24542c3bb3f0a70e16cf83e218aa6
- author
- brice · · 2026-07-10 06:17 UTC
- parent
- 0ae8e2a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +83 -9
M internal/usecase/daemon/floating_render_test.go +19 -0
| @@ -63,6 +63,25 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) { | |||
| 63 | 63 | require.Equal(t, 1, damage[0].Height) | |
| 64 | 64 | } | |
| 65 | 65 | ||
| 66 | + | func TestComposeFloatingFrameRendersPaneOwnedCommandFallback(t *testing.T) { | |
| 67 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 68 | + | d.shell = "/usr/bin/fish" | |
| 69 | + | p := newPane("floating", nil, domain.Size{Cols: 6, Rows: 3}) | |
| 70 | + | p.title.displayFallback = floatingCommandFallback("btop --utf", d.shell) | |
| 71 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, true)) | |
| 72 | + | ||
| 73 | + | base := renderer.NewFrame(40, 12) | |
| 74 | + | content := domain.Rect{Y: 1, Width: 40, Height: 10} | |
| 75 | + | cfg := domain.FloatingConfig{Width: 80, Height: 80} | |
| 76 | + | frame, _ := composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, false) | |
| 77 | + | geometry := calculateFloatingGeometry(content, cfg) | |
| 78 | + | var gotTitle strings.Builder | |
| 79 | + | for x := geometry.Bounds.X + 2; x < geometry.Bounds.X+geometry.Bounds.Width-2; x++ { | |
| 80 | + | gotTitle.WriteRune(frame.At(x, geometry.Bounds.Y).Rune) | |
| 81 | + | } | |
| 82 | + | require.Equal(t, "btop", strings.TrimRight(gotTitle.String(), "─")) | |
| 83 | + | } | |
| 84 | + | ||
| 66 | 85 | func TestComposeFloatingFrameSynchronizesWithPTYReader(t *testing.T) { | |
| 67 | 86 | p := newPane("floating", nil, domain.Size{Cols: 80, Rows: 24}) | |
| 68 | 87 | base := renderer.NewFrame(80, 24) | |
| @@ -63,6 +63,25 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) { | |||||
| 63 | require.Equal(t, 1, damage[0].Height) | 63 | require.Equal(t, 1, damage[0].Height) | ||
| 64 | } | 64 | } | ||
| 65 | 65 | ||||
| 66 | + | func TestComposeFloatingFrameRendersPaneOwnedCommandFallback(t *testing.T) { | |||
| 67 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 68 | + | d.shell = "/usr/bin/fish" | |||
| 69 | + | p := newPane("floating", nil, domain.Size{Cols: 6, Rows: 3}) | |||
| 70 | + | p.title.displayFallback = floatingCommandFallback("btop --utf", d.shell) | |||
| 71 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, true)) | |||
| 72 | + | ||||
| 73 | + | base := renderer.NewFrame(40, 12) | |||
| 74 | + | content := domain.Rect{Y: 1, Width: 40, Height: 10} | |||
| 75 | + | cfg := domain.FloatingConfig{Width: 80, Height: 80} | |||
| 76 | + | frame, _ := composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, false) | |||
| 77 | + | geometry := calculateFloatingGeometry(content, cfg) | |||
| 78 | + | var gotTitle strings.Builder | |||
| 79 | + | for x := geometry.Bounds.X + 2; x < geometry.Bounds.X+geometry.Bounds.Width-2; x++ { | |||
| 80 | + | gotTitle.WriteRune(frame.At(x, geometry.Bounds.Y).Rune) | |||
| 81 | + | } | |||
| 82 | + | require.Equal(t, "btop", strings.TrimRight(gotTitle.String(), "─")) | |||
| 83 | + | } | |||
| 84 | + | ||||
| 66 | func TestComposeFloatingFrameSynchronizesWithPTYReader(t *testing.T) { | 85 | func TestComposeFloatingFrameSynchronizesWithPTYReader(t *testing.T) { | ||
| 67 | p := newPane("floating", nil, domain.Size{Cols: 80, Rows: 24}) | 86 | p := newPane("floating", nil, domain.Size{Cols: 80, Rows: 24}) | ||
| 68 | base := renderer.NewFrame(80, 24) | 87 | base := renderer.NewFrame(80, 24) | ||
M internal/usecase/daemon/pane_titles.go +14 -8
| @@ -43,6 +43,18 @@ func (p *pane) displayTitleLocked() string { | |||
| 43 | 43 | return p.formattedTitleLocked(p.title.displayFallback) | |
| 44 | 44 | } | |
| 45 | 45 | ||
| 46 | + | // setDisplayFallback updates the fallback owned by a pane. It must not damage | |
| 47 | + | // cached composition unless that fallback changes the displayed title. | |
| 48 | + | func (p *pane) setDisplayFallback(fallback string) { | |
| 49 | + | p.mu.Lock() | |
| 50 | + | oldTitle := p.displayTitleLocked() | |
| 51 | + | p.title.displayFallback = fallback | |
| 52 | + | if oldTitle != p.displayTitleLocked() { | |
| 53 | + | p.title.generation++ | |
| 54 | + | } | |
| 55 | + | p.mu.Unlock() | |
| 56 | + | } | |
| 57 | + | ||
| 46 | 58 | // refreshTerminalTitleLocked synchronizes the title retained by the VT parser. | |
| 47 | 59 | // It must be called while p.mu is held, including by ptyReader around Write. | |
| 48 | 60 | func (p *pane) refreshTerminalTitleLocked() { | |
| @@ -85,6 +97,7 @@ func (d *Daemon) refreshPaneTitleCached(sess *session, id layout.PaneID, force b | |||
| 85 | 97 | if p == nil { | |
| 86 | 98 | return fallback | |
| 87 | 99 | } | |
| 100 | + | p.setDisplayFallback(fallback) | |
| 88 | 101 | return d.refreshPaneDisplayTitle(sess, p, force) | |
| 89 | 102 | } | |
| 90 | 103 | ||
| @@ -92,19 +105,13 @@ func (d *Daemon) refreshPaneTitleCached(sess *session, id layout.PaneID, force b | |||
| 92 | 105 | // display title. It deliberately releases pane.mu before querying the PTY so | |
| 93 | 106 | // callers that already render under pane locking cannot recurse on that lock. | |
| 94 | 107 | func (d *Daemon) refreshPaneDisplayTitle(_ *session, p *pane, force bool) string { | |
| 95 | - | fallback := d.paneTitleFallback() | |
| 96 | 108 | if p == nil { | |
| 97 | - | return fallback | |
| 109 | + | return d.paneTitleFallback() | |
| 98 | 110 | } | |
| 99 | 111 | now := d.clock.Now() | |
| 100 | 112 | p.mu.Lock() | |
| 101 | 113 | if !force && p.title.processNameValid && now.Sub(p.title.processNameAt) < paneTitleCacheTTL { | |
| 102 | - | oldTitle := p.displayTitleLocked() | |
| 103 | - | p.title.displayFallback = fallback | |
| 104 | 114 | title := p.displayTitleLocked() | |
| 105 | - | if oldTitle != title { | |
| 106 | - | p.title.generation++ | |
| 107 | - | } | |
| 108 | 115 | p.mu.Unlock() | |
| 109 | 116 | return title | |
| 110 | 117 | } | |
| @@ -127,7 +134,6 @@ func (d *Daemon) refreshPaneDisplayTitle(_ *session, p *pane, force bool) string | |||
| 127 | 134 | p.title.processName = processName | |
| 128 | 135 | p.title.processNameAt = now | |
| 129 | 136 | p.title.processNameValid = true | |
| 130 | - | p.title.displayFallback = fallback | |
| 131 | 137 | title := p.displayTitleLocked() | |
| 132 | 138 | if oldTitle != title { | |
| 133 | 139 | p.title.generation++ | |
| @@ -43,6 +43,18 @@ func (p *pane) displayTitleLocked() string { | |||||
| 43 | return p.formattedTitleLocked(p.title.displayFallback) | 43 | return p.formattedTitleLocked(p.title.displayFallback) | ||
| 44 | } | 44 | } | ||
| 45 | 45 | ||||
| 46 | + | // setDisplayFallback updates the fallback owned by a pane. It must not damage | |||
| 47 | + | // cached composition unless that fallback changes the displayed title. | |||
| 48 | + | func (p *pane) setDisplayFallback(fallback string) { | |||
| 49 | + | p.mu.Lock() | |||
| 50 | + | oldTitle := p.displayTitleLocked() | |||
| 51 | + | p.title.displayFallback = fallback | |||
| 52 | + | if oldTitle != p.displayTitleLocked() { | |||
| 53 | + | p.title.generation++ | |||
| 54 | + | } | |||
| 55 | + | p.mu.Unlock() | |||
| 56 | + | } | |||
| 57 | + | ||||
| 46 | // refreshTerminalTitleLocked synchronizes the title retained by the VT parser. | 58 | // refreshTerminalTitleLocked synchronizes the title retained by the VT parser. | ||
| 47 | // It must be called while p.mu is held, including by ptyReader around Write. | 59 | // It must be called while p.mu is held, including by ptyReader around Write. | ||
| 48 | func (p *pane) refreshTerminalTitleLocked() { | 60 | func (p *pane) refreshTerminalTitleLocked() { | ||
| @@ -85,6 +97,7 @@ func (d *Daemon) refreshPaneTitleCached(sess *session, id layout.PaneID, force b | |||||
| 85 | if p == nil { | 97 | if p == nil { | ||
| 86 | return fallback | 98 | return fallback | ||
| 87 | } | 99 | } | ||
| 100 | + | p.setDisplayFallback(fallback) | |||
| 88 | return d.refreshPaneDisplayTitle(sess, p, force) | 101 | return d.refreshPaneDisplayTitle(sess, p, force) | ||
| 89 | } | 102 | } | ||
| 90 | 103 | ||||
| @@ -92,19 +105,13 @@ func (d *Daemon) refreshPaneTitleCached(sess *session, id layout.PaneID, force b | |||||
| 92 | // display title. It deliberately releases pane.mu before querying the PTY so | 105 | // display title. It deliberately releases pane.mu before querying the PTY so | ||
| 93 | // callers that already render under pane locking cannot recurse on that lock. | 106 | // callers that already render under pane locking cannot recurse on that lock. | ||
| 94 | func (d *Daemon) refreshPaneDisplayTitle(_ *session, p *pane, force bool) string { | 107 | func (d *Daemon) refreshPaneDisplayTitle(_ *session, p *pane, force bool) string { | ||
| 95 | - | fallback := d.paneTitleFallback() | |||
| 96 | if p == nil { | 108 | if p == nil { | ||
| 97 | - | return fallback | 109 | + | return d.paneTitleFallback() |
| 98 | } | 110 | } | ||
| 99 | now := d.clock.Now() | 111 | now := d.clock.Now() | ||
| 100 | p.mu.Lock() | 112 | p.mu.Lock() | ||
| 101 | if !force && p.title.processNameValid && now.Sub(p.title.processNameAt) < paneTitleCacheTTL { | 113 | if !force && p.title.processNameValid && now.Sub(p.title.processNameAt) < paneTitleCacheTTL { | ||
| 102 | - | oldTitle := p.displayTitleLocked() | |||
| 103 | - | p.title.displayFallback = fallback | |||
| 104 | title := p.displayTitleLocked() | 114 | title := p.displayTitleLocked() | ||
| 105 | - | if oldTitle != title { | |||
| 106 | - | p.title.generation++ | |||
| 107 | - | } | |||
| 108 | p.mu.Unlock() | 115 | p.mu.Unlock() | ||
| 109 | return title | 116 | return title | ||
| 110 | } | 117 | } | ||
| @@ -127,7 +134,6 @@ func (d *Daemon) refreshPaneDisplayTitle(_ *session, p *pane, force bool) string | |||||
| 127 | p.title.processName = processName | 134 | p.title.processName = processName | ||
| 128 | p.title.processNameAt = now | 135 | p.title.processNameAt = now | ||
| 129 | p.title.processNameValid = true | 136 | p.title.processNameValid = true | ||
| 130 | - | p.title.displayFallback = fallback | |||
| 131 | title := p.displayTitleLocked() | 137 | title := p.displayTitleLocked() | ||
| 132 | if oldTitle != title { | 138 | if oldTitle != title { | ||
| 133 | p.title.generation++ | 139 | p.title.generation++ | ||
M internal/usecase/daemon/pane_titles_test.go +50 -1
| @@ -96,6 +96,55 @@ func TestRefreshPaneTitleCachesByTTLAndRefreshesOnFocus(t *testing.T) { | |||
| 96 | 96 | require.Equal(t, int32(2), calls.Load(), "focus refresh should bypass TTL") | |
| 97 | 97 | } | |
| 98 | 98 | ||
| 99 | + | func TestRefreshFloatingPaneTitlePreservesConfiguredCommandFallback(t *testing.T) { | |
| 100 | + | pty := portsmocks.NewMockPTY(t) | |
| 101 | + | pty.EXPECT().ForegroundPgid().Return(0, errors.New("no foreground process")).Twice() | |
| 102 | + | clk := portsmocks.NewMockClock(t) | |
| 103 | + | clk.EXPECT().Now().Return(time.Time{}).Maybe() | |
| 104 | + | d := newTestDaemon(t, nil, clk) | |
| 105 | + | d.shell = "/usr/bin/fish" | |
| 106 | + | d.procComm = func(int) (string, error) { return "", errors.New("unused") } | |
| 107 | + | p := newPane("floating", pty, domain.Size{Cols: 20, Rows: 5}) | |
| 108 | + | p.title.displayFallback = floatingCommandFallback("btop --utf", d.shell) | |
| 109 | + | ||
| 110 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, false)) | |
| 111 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, false), "cached refresh must retain the pane-owned fallback") | |
| 112 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, true), "process refresh must retain the pane-owned fallback") | |
| 113 | + | p.mu.Lock() | |
| 114 | + | require.Equal(t, "btop", p.title.displayFallback) | |
| 115 | + | p.mu.Unlock() | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | func TestRefreshFloatingPaneTitleUsesShellFallbackForEmptyCommand(t *testing.T) { | |
| 119 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 120 | + | d.shell = "/usr/local/bin/zsh" | |
| 121 | + | p := newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}) | |
| 122 | + | p.title.displayFallback = floatingCommandFallback("", d.shell) | |
| 123 | + | ||
| 124 | + | require.Equal(t, "zsh", d.refreshPaneDisplayTitle(nil, p, true)) | |
| 125 | + | } | |
| 126 | + | ||
| 127 | + | func TestRefreshPaneTitleUpdatesNormalPaneShellFallback(t *testing.T) { | |
| 128 | + | d, sess, _, _ := newManualSessionWithPTYs(t, nil) | |
| 129 | + | p := sess.activeTab().focusedPane() | |
| 130 | + | ||
| 131 | + | d.shell = "/usr/bin/fish" | |
| 132 | + | require.Equal(t, "fish", d.refreshPaneTitle(sess, "pane-1")) | |
| 133 | + | p.mu.Lock() | |
| 134 | + | firstGeneration := p.title.generation | |
| 135 | + | p.mu.Unlock() | |
| 136 | + | require.Equal(t, "fish", d.refreshPaneTitle(sess, "pane-1")) | |
| 137 | + | p.mu.Lock() | |
| 138 | + | require.Equal(t, firstGeneration, p.title.generation, "unchanged displayed title must not damage the title row") | |
| 139 | + | p.mu.Unlock() | |
| 140 | + | ||
| 141 | + | d.shell = "/bin/zsh" | |
| 142 | + | require.Equal(t, "zsh", d.refreshPaneTitle(sess, "pane-1")) | |
| 143 | + | p.mu.Lock() | |
| 144 | + | require.Equal(t, firstGeneration+1, p.title.generation) | |
| 145 | + | p.mu.Unlock() | |
| 146 | + | } | |
| 147 | + | ||
| 99 | 148 | func TestRefreshPaneTitleLookupFailureKeepsProcessNameEmpty(t *testing.T) { | |
| 100 | 149 | pty := portsmocks.NewMockPTY(t) | |
| 101 | 150 | pty.EXPECT().ForegroundPgid().Return(0, errors.New("no foreground process")).Twice() | |
| @@ -120,5 +169,5 @@ func TestRefreshPaneTitleLookupFailureKeepsProcessNameEmpty(t *testing.T) { | |||
| 120 | 169 | p.screen.Write([]byte("\x1b]2;\a")) | |
| 121 | 170 | p.refreshTerminalTitleLocked() | |
| 122 | 171 | p.mu.Unlock() | |
| 123 | - | require.Equal(t, "fish", d.refreshPaneDisplayTitle(sess, p, true)) | |
| 172 | + | require.Equal(t, "sh", d.refreshPaneDisplayTitle(sess, p, true), "pointer refresh must retain its pane-owned fallback") | |
| 124 | 173 | } | |
| @@ -96,6 +96,55 @@ func TestRefreshPaneTitleCachesByTTLAndRefreshesOnFocus(t *testing.T) { | |||||
| 96 | require.Equal(t, int32(2), calls.Load(), "focus refresh should bypass TTL") | 96 | require.Equal(t, int32(2), calls.Load(), "focus refresh should bypass TTL") | ||
| 97 | } | 97 | } | ||
| 98 | 98 | ||||
| 99 | + | func TestRefreshFloatingPaneTitlePreservesConfiguredCommandFallback(t *testing.T) { | |||
| 100 | + | pty := portsmocks.NewMockPTY(t) | |||
| 101 | + | pty.EXPECT().ForegroundPgid().Return(0, errors.New("no foreground process")).Twice() | |||
| 102 | + | clk := portsmocks.NewMockClock(t) | |||
| 103 | + | clk.EXPECT().Now().Return(time.Time{}).Maybe() | |||
| 104 | + | d := newTestDaemon(t, nil, clk) | |||
| 105 | + | d.shell = "/usr/bin/fish" | |||
| 106 | + | d.procComm = func(int) (string, error) { return "", errors.New("unused") } | |||
| 107 | + | p := newPane("floating", pty, domain.Size{Cols: 20, Rows: 5}) | |||
| 108 | + | p.title.displayFallback = floatingCommandFallback("btop --utf", d.shell) | |||
| 109 | + | ||||
| 110 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, false)) | |||
| 111 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, false), "cached refresh must retain the pane-owned fallback") | |||
| 112 | + | require.Equal(t, "btop", d.refreshPaneDisplayTitle(nil, p, true), "process refresh must retain the pane-owned fallback") | |||
| 113 | + | p.mu.Lock() | |||
| 114 | + | require.Equal(t, "btop", p.title.displayFallback) | |||
| 115 | + | p.mu.Unlock() | |||
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | func TestRefreshFloatingPaneTitleUsesShellFallbackForEmptyCommand(t *testing.T) { | |||
| 119 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 120 | + | d.shell = "/usr/local/bin/zsh" | |||
| 121 | + | p := newPane("floating", nil, domain.Size{Cols: 20, Rows: 5}) | |||
| 122 | + | p.title.displayFallback = floatingCommandFallback("", d.shell) | |||
| 123 | + | ||||
| 124 | + | require.Equal(t, "zsh", d.refreshPaneDisplayTitle(nil, p, true)) | |||
| 125 | + | } | |||
| 126 | + | ||||
| 127 | + | func TestRefreshPaneTitleUpdatesNormalPaneShellFallback(t *testing.T) { | |||
| 128 | + | d, sess, _, _ := newManualSessionWithPTYs(t, nil) | |||
| 129 | + | p := sess.activeTab().focusedPane() | |||
| 130 | + | ||||
| 131 | + | d.shell = "/usr/bin/fish" | |||
| 132 | + | require.Equal(t, "fish", d.refreshPaneTitle(sess, "pane-1")) | |||
| 133 | + | p.mu.Lock() | |||
| 134 | + | firstGeneration := p.title.generation | |||
| 135 | + | p.mu.Unlock() | |||
| 136 | + | require.Equal(t, "fish", d.refreshPaneTitle(sess, "pane-1")) | |||
| 137 | + | p.mu.Lock() | |||
| 138 | + | require.Equal(t, firstGeneration, p.title.generation, "unchanged displayed title must not damage the title row") | |||
| 139 | + | p.mu.Unlock() | |||
| 140 | + | ||||
| 141 | + | d.shell = "/bin/zsh" | |||
| 142 | + | require.Equal(t, "zsh", d.refreshPaneTitle(sess, "pane-1")) | |||
| 143 | + | p.mu.Lock() | |||
| 144 | + | require.Equal(t, firstGeneration+1, p.title.generation) | |||
| 145 | + | p.mu.Unlock() | |||
| 146 | + | } | |||
| 147 | + | ||||
| 99 | func TestRefreshPaneTitleLookupFailureKeepsProcessNameEmpty(t *testing.T) { | 148 | func TestRefreshPaneTitleLookupFailureKeepsProcessNameEmpty(t *testing.T) { | ||
| 100 | pty := portsmocks.NewMockPTY(t) | 149 | pty := portsmocks.NewMockPTY(t) | ||
| 101 | pty.EXPECT().ForegroundPgid().Return(0, errors.New("no foreground process")).Twice() | 150 | pty.EXPECT().ForegroundPgid().Return(0, errors.New("no foreground process")).Twice() | ||
| @@ -120,5 +169,5 @@ func TestRefreshPaneTitleLookupFailureKeepsProcessNameEmpty(t *testing.T) { | |||||
| 120 | p.screen.Write([]byte("\x1b]2;\a")) | 169 | p.screen.Write([]byte("\x1b]2;\a")) | ||
| 121 | p.refreshTerminalTitleLocked() | 170 | p.refreshTerminalTitleLocked() | ||
| 122 | p.mu.Unlock() | 171 | p.mu.Unlock() | ||
| 123 | - | require.Equal(t, "fish", d.refreshPaneDisplayTitle(sess, p, true)) | 172 | + | require.Equal(t, "sh", d.refreshPaneDisplayTitle(sess, p, true), "pointer refresh must retain its pane-owned fallback") |
| 124 | } | 173 | } | ||