Suppress right-edge scroll bar on TUI alt-screen and add disable setting (fixes 2728) - #2729
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a global and per-workspace setting to show/hide the terminal right-edge scrollbar, plus UI, localization, settings/schema/docs, session persistence, context-menu wiring, and terminal view geometry/visibility synchronization. Changes
Sequence DiagramsequenceDiagram
participant User
participant ContentView
participant TabManager
participant Workspace
participant Defaults as UserDefaults
participant Terminal as GhosttyTerminalView
User->>ContentView: open context menu / toggle "Hide Terminal Scroll Bar"
ContentView->>TabManager: setWorkspaceTerminalScrollBarHidden(tabId, hidden)
TabManager->>Workspace: setTerminalScrollBarHidden(hidden)
Workspace->>Workspace: update published state & post notification
Defaults->>Terminal: TerminalScrollBarSettings.isVisible()
Workspace->>Terminal: workspace notification (terminalScrollBarHidden changed)
Terminal->>Terminal: shouldShowTerminalScrollBar() (global + workspace + content)
Terminal->>Terminal: scrollView.tile() / synchronizeGeometryAndContent()
Terminal->>User: redraw terminal with updated scrollbar visibility
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes the right-edge scrollbar overlap for full-screen TUI apps ( Confidence Score: 5/5Safe to merge; all findings are P2 style/robustness suggestions with no blocking correctness issues. The core logic — suppressing the scrollbar on total <= len and reserving gutter width — is sound. Persistence, settings wiring, localization, schema, and docs are all consistent. The three comments are non-blocking: overly broad UserDefaults observer, initial-nil scrollbar returning false, and the removed isolation comment for handlePreferredScrollerStyleChange. Sources/GhosttyTerminalView.swift — broad UserDefaults observer and initial-nil scrollbar behavior worth a second look.
|
| Filename | Overview |
|---|---|
| Sources/GhosttyTerminalView.swift | Core scrollbar suppression logic: adds shouldShowTerminalScrollBar(), gutter reservation, workspace/settings lookup, and a broad UserDefaults.didChangeNotification observer; three P2 concerns noted. |
| Sources/Workspace.swift | Adds terminalScrollBarHidden: Bool published property with setTerminalScrollBarHidden() that guards duplicate writes and schedules geometry reconcile; serialization correctly stores nil for the default (non-hidden) case. |
| Sources/SessionPersistence.swift | Adds optional terminalScrollBarHidden: Bool? to SessionWorkspaceSnapshot; backward-compatible with nil defaulting to false on restore. |
| Sources/cmuxApp.swift | Adds TerminalScrollBarSettings enum with isVisible() helper and the Settings UI toggle wired to @AppStorage; reset path correctly updated. |
| Sources/ContentView.swift | Adds Workspace Settings context menu item with checkmark toggling; context menus are ephemeral so stale-state impact of reading tabManager.tabs in the label closure is negligible. |
| Sources/KeyboardShortcutSettingsFileStore.swift | Wires terminal.showScrollBar into the settings file parser; follows the existing section-parsing pattern. |
| Sources/TabManager.swift | Adds setWorkspaceTerminalScrollBarHidden() forwarding method and includes terminalScrollBarHidden in the workspace hasher. |
| web/data/cmux-settings.schema.json | Adds terminal section with showScrollBar boolean property consistent with default true and the implementation. |
Sequence Diagram
sequenceDiagram
participant G as Ghostty (libghostty)
participant SV as GhosttySurfaceScrollView
participant W as Workspace
participant UD as UserDefaults
Note over G,SV: Scrollbar telemetry path
G->>SV: handleScrollbarUpdate(notification)
SV->>SV: shouldShowTerminalScrollBar() [before]
SV->>SV: surfaceView.scrollbar = scrollbar
SV->>SV: shouldShowTerminalScrollBar() [after]
alt visibility changed
SV->>SV: synchronizeGeometryAndContent()
else no change
SV->>SV: synchronizeScrollView()
end
Note over W,SV: Per-workspace toggle path
W->>W: setTerminalScrollBarHidden(hidden)
W->>W: scheduleTerminalGeometryReconcile()
W-->>SV: layout() triggered by AppKit
SV->>SV: synchronizeGeometryAndContent()
SV->>W: owningWorkspace()?.terminalScrollBarHidden
Note over UD,SV: Global settings path
UD->>SV: UserDefaults.didChangeNotification (any key)
SV->>SV: handleTerminalScrollBarPreferenceChange()
SV->>SV: synchronizeGeometryAndContent()
SV->>UD: TerminalScrollBarSettings.isVisible()
Reviews (1): Last reviewed commit: "Suppress terminal scrollbar for TUI work..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
11277-11292:⚠️ Potential issue | 🟠 MajorCompare against the current scroller state, not
surfaceView.scrollbar.
GhosttyNSView.flushPendingScrollbar()already writes the new telemetry before posting this notification, so Line 11281 is reading the new state, not the previous one. That makeswasVisibleandisVisibleequal, so entering/leaving alt-screen or receiving the first scrollback packet never triggerssynchronizeGeometryAndContent(). The gutter/scroller state then stays stale until some unrelated layout pass happens.Suggested fix
private func handleScrollbarUpdate(_ notification: Notification) { guard let scrollbar = notification.userInfo?[GhosttyNotificationKey.scrollbar] as? GhosttyScrollbar else { return } - let wasVisible = shouldShowTerminalScrollBar() + let wasVisible = scrollView.hasVerticalScroller if pendingExplicitWheelScroll { userScrolledAwayFromBottom = scrollbar.offset + scrollbar.len < scrollbar.total allowExplicitScrollbarSync = true pendingExplicitWheelScroll = false } surfaceView.scrollbar = scrollbar - let isVisible = shouldShowTerminalScrollBar() + let isVisible = terminalScrollBarAllowedBySettings() && scrollbar.total > scrollbar.len if wasVisible != isVisible { _ = synchronizeGeometryAndContent() return } synchronizeScrollView() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 11277 - 11292, The handler reads the "new" scroller state via shouldShowTerminalScrollBar() (which reflects flushed telemetry) so wasVisible is wrong; instead capture the previous scroller state from surfaceView.scrollbar before applying the notification. In handleScrollbarUpdate, store the existing surfaceView.scrollbar (e.g. previousScrollbar = surfaceView.scrollbar) and compute wasVisible from that previousScrollbar, then assign surfaceView.scrollbar = scrollbar and compute isVisible via shouldShowTerminalScrollBar() as before; keep the existing pendingExplicitWheelScroll / userScrolledAwayFromBottom / allowExplicitScrollbarSync logic intact.
🧹 Nitpick comments (1)
Sources/Workspace.swift (1)
6491-6491: Harden withprivate(set)to prevent accidental direct mutations.The backing
terminalScrollBarHiddenproperty is still writable from anywhere in the file. While a search confirms no current direct assignments bypasssetTerminalScrollBarHidden(_:), make the propertyprivate(set)to enforce that the setter—which triggersscheduleTerminalGeometryReconcile()—is always the path for mutations. This prevents future changes from accidentally updating the state without resizing the Ghostty surface.Also applies to: 7529-7533
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` at line 6491, Make the Published property terminalScrollBarHidden read-only externally by changing its declaration to use private(set) so callers cannot directly assign it; ensure all mutations go through the existing setter method setTerminalScrollBarHidden(_:) which calls scheduleTerminalGeometryReconcile(). Also apply the same private(set) hardening to the other similar Published property controlled by its own setter (the one referenced in the review at lines 7529-7533) so that all external changes must use the corresponding setter that triggers geometry reconciliation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 11277-11292: The handler reads the "new" scroller state via
shouldShowTerminalScrollBar() (which reflects flushed telemetry) so wasVisible
is wrong; instead capture the previous scroller state from surfaceView.scrollbar
before applying the notification. In handleScrollbarUpdate, store the existing
surfaceView.scrollbar (e.g. previousScrollbar = surfaceView.scrollbar) and
compute wasVisible from that previousScrollbar, then assign
surfaceView.scrollbar = scrollbar and compute isVisible via
shouldShowTerminalScrollBar() as before; keep the existing
pendingExplicitWheelScroll / userScrolledAwayFromBottom /
allowExplicitScrollbarSync logic intact.
---
Nitpick comments:
In `@Sources/Workspace.swift`:
- Line 6491: Make the Published property terminalScrollBarHidden read-only
externally by changing its declaration to use private(set) so callers cannot
directly assign it; ensure all mutations go through the existing setter method
setTerminalScrollBarHidden(_:) which calls scheduleTerminalGeometryReconcile().
Also apply the same private(set) hardening to the other similar Published
property controlled by its own setter (the one referenced in the review at lines
7529-7533) so that all external changes must use the corresponding setter that
triggers geometry reconciliation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b85456f9-294e-42fc-9f60-e4d8bb1a9a51
📒 Files selected for processing (10)
Resources/Localizable.xcstringsSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/Workspace.swiftSources/cmuxApp.swiftweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux-settings.schema.json
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:13285">
P2: `TabItemView` now reads `tabManager` from its body via `allTargetWorkspacesHideTerminalScrollBar`, which breaks the documented rule to avoid `tabManager` in the body (equatable typing hot path). Precompute this flag outside the body and pass it in as a stored parameter instead.</violation>
</file>
<file name="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:9066">
P2: `UserDefaults.didChangeNotification` fires for every defaults write (workspace renames, theme changes, sidebar settings, etc.), not just scroll-bar preference changes. Each `GhosttySurfaceScrollView` instance will run a full `synchronizeGeometryAndContent()` pass on every unrelated write. In a session with many splits this creates unnecessary layout churn. Filter by checking whether the scroll-bar key actually changed, or use `KVO`/`AppStorage` scoped to the specific key.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
9973-10027:⚠️ Potential issue | 🟠 MajorRecompute the current hidden state in the handler instead of trusting the snapshotted Bool.
allContextMenuWorkspacesHideTerminalScrollBaris derived once inVerticalTabsSidebarand then reused ascurrentlyHiddenfor the mutation. These rows refresh through their own observation path, so that snapshot can lag the realterminalScrollBarHiddenvalues and make the toggle flip the selected workspaces the wrong way. The action should derive the current aggregate state fromtargetIdsright before mutating.Suggested change
- toggleWorkspaceTerminalScrollBarHidden( - targetIds: targetIds, - currentlyHidden: allContextMenuWorkspacesHideTerminalScrollBar - ) + toggleWorkspaceTerminalScrollBarHidden(targetIds: targetIds) ... - private func toggleWorkspaceTerminalScrollBarHidden(targetIds: [UUID], currentlyHidden: Bool) { - let hideScrollBar = !currentlyHidden + private func toggleWorkspaceTerminalScrollBarHidden(targetIds: [UUID]) { + let workspacesById = Dictionary(uniqueKeysWithValues: tabManager.tabs.map { ($0.id, $0) }) + let currentlyHidden = !targetIds.isEmpty && + targetIds.allSatisfy { workspacesById[$0]?.terminalScrollBarHidden == true } + let hideScrollBar = !currentlyHidden for targetId in targetIds { tabManager.setWorkspaceTerminalScrollBarHidden(tabId: targetId, hidden: hideScrollBar) } }Based on learnings: In
TabItemViewinContentView.swift, useEquatable+.equatable()and keep reactive row state off parent-diff-only inputs.Also applies to: 13286-13301, 14006-14011
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 9973 - 10027, The computed Bool allContextMenuWorkspacesHideTerminalScrollBar should not be captured and used as the authoritative "currentlyHidden" in the toggle handler; instead recompute the aggregate from the actual workspace IDs right before mutating (e.g. use contextMenuWorkspaceIds.allSatisfy { tabsById[$0]?.terminalScrollBarHidden == true } inside the action that flips terminalScrollBarHidden), update VerticalTabsSidebar/TabItemView callsites to use that recomputed value in the mutation, and make TabItemView inputs equatable (use Equatable + .equatable()) so row state is driven by its own observed values rather than parent snapshot-only inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 9066-9072: The view currently only observes
TerminalScrollBarSettings.didChangeNotification; also add an observer for
workspace-level changes to immediately react when
Workspace.terminalScrollBarHidden is toggled (e.g., observe the Workspace change
notification or KVO the workspace's terminalScrollBarHidden) and invoke
handleTerminalScrollBarPreferenceChange() so updateNSView/resync logic runs
immediately; mirror this change wherever the same observer block is added (the
other occurrence around lines 11351-11380) and ensure the new observer is
cleaned up alongside the existing ones.
---
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 9973-10027: The computed Bool
allContextMenuWorkspacesHideTerminalScrollBar should not be captured and used as
the authoritative "currentlyHidden" in the toggle handler; instead recompute the
aggregate from the actual workspace IDs right before mutating (e.g. use
contextMenuWorkspaceIds.allSatisfy { tabsById[$0]?.terminalScrollBarHidden ==
true } inside the action that flips terminalScrollBarHidden), update
VerticalTabsSidebar/TabItemView callsites to use that recomputed value in the
mutation, and make TabItemView inputs equatable (use Equatable + .equatable())
so row state is driven by its own observed values rather than parent
snapshot-only inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0793045f-26d6-4e13-9900-f8cd1b63e700
📒 Files selected for processing (5)
Sources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Workspace.swiftSources/cmuxApp.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- Sources/KeyboardShortcutSettingsFileStore.swift
- Sources/Workspace.swift
- Sources/cmuxApp.swift
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:9067">
P2: Listening only to `TerminalScrollBarSettings.didChangeNotification` can miss scrollbar preference changes from direct `@AppStorage` writes (e.g. settings reset), leaving terminal scrollbar geometry stale until another relayout trigger occurs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Follow-up push: Addressed the remaining review feedback:
All open review threads on these items are resolved. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 9401-9406: When reattaching a TerminalSurface in
attachSurface(_:), after setting cachedOwningWorkspace and calling
updateWorkspaceTerminalScrollBarObserver(workspace) immediately apply the
current terminalScrollBarHidden value for that workspace so the gutter/scroller
state is updated right away; locate the logic that applies
terminalScrollBarHidden elsewhere (the code that reads terminalScrollBarHidden
and updates surfaceView/gutter visibility) and invoke it here (e.g. call the
function that applies workspace scroll bar visibility or directly set
surfaceView visibility using the terminalScrollBarHidden value for
cachedOwningWorkspace) so the newly attached surface reflects the correct
workspace state immediately.
- Around line 11395-11420: The gutter/PTY width isn't reconciled when Ghostty's
scrollbar config changes; update refreshHostBackgroundAfterGhosttyConfigReload()
so that after calling synchronizeScrollbarAppearance() it also forces a
geometry/layout reconciliation driven by
terminalScrollBarAllowedBySettings()/shouldShowTerminalScrollBar()/terminalScrollBarReservedWidth():
e.g. dispatch to main thread and call surfaceView.setNeedsLayout() (or
invalidateIntrinsicContentSize()/layoutSubtreeIfNeeded()) and notify the
workspace (owningWorkspace()?.updateTerminalLayout() or similar) so the
gutter/PTY width is recomputed immediately when scrollbar config reloads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2e328e7-fa7f-4378-963e-4e893c85ce82
📒 Files selected for processing (4)
Sources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Workspace.swiftSources/cmuxApp.swift
✅ Files skipped from review due to trivial changes (1)
- Sources/ContentView.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/cmuxApp.swift
- Sources/Workspace.swift
|
Follow-up push: Addressed the two new GitHub review findings:
The new review threads are resolved. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/Workspace.swift (1)
6485-6487: Mark this notification constantnonisolatedto avoid forcing observer registration code into main-actor context.
Workspace.terminalScrollBarHiddenDidChangeNotificationis implicitly main-actor-isolated because it's astatic letin an@MainActorclass. SinceNotification.Nameis a value type andSendable, marking itnonisolated static letallowsGhosttyTerminalViewand other non-main-actor code to reference it without async overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 6485 - 6487, Workspace.terminalScrollBarHiddenDidChangeNotification is currently main-actor-isolated because it's a static let on an `@MainActor` class; change its declaration to be nonisolated (i.e., declare it as nonisolated static let terminalScrollBarHiddenDidChangeNotification = ...) so non-main-actor code like GhosttyTerminalView can reference this Notification.Name without async/await or hopping to the main actor; keep the value type and initialization the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/ContentView.swift`:
- Around line 13173-13178: The TabItemView currently reads tabManager.tabs
inside its private helper allTargetWorkspacesHideTerminalScrollBar, violating
the .equatable() / Equatable precomputed-inputs rule and causing stale
checkmarks; move that boolean computation into VerticalTabsSidebar where you
have access to the tab list, compute a single Bool (e.g.
allTargetsHideTerminalScrollBar) for each TabItemView, add that Bool as a stored
initializer parameter to TabItemView, include it in TabItemView’s Equatable ==
comparison, and remove any direct reads of tabManager or notificationStore from
TabItemView’s body (replace calls to allTargetWorkspacesHideTerminalScrollBar
and similar logic around lines 13173 and 13285-13299 with the injected
parameter).
In `@Sources/GhosttyTerminalView.swift`:
- Around line 11351-11359: The guard in updateWorkspaceTerminalScrollBarObserver
currently can early-return when both observedWorkspaceTerminalScrollBar and
workspace are nil while workspaceTerminalScrollBarObserver is non-nil, skipping
cleanup; change the early-return logic so it only returns when the
observedWorkspaceTerminalScrollBar === workspace AND
workspaceTerminalScrollBarObserver == nil (i.e., nothing to do), ensuring that
when workspaceTerminalScrollBarObserver is non-nil you proceed to remove the
observer and set workspaceTerminalScrollBarObserver = nil.
---
Nitpick comments:
In `@Sources/Workspace.swift`:
- Around line 6485-6487: Workspace.terminalScrollBarHiddenDidChangeNotification
is currently main-actor-isolated because it's a static let on an `@MainActor`
class; change its declaration to be nonisolated (i.e., declare it as nonisolated
static let terminalScrollBarHiddenDidChangeNotification = ...) so non-main-actor
code like GhosttyTerminalView can reference this Notification.Name without
async/await or hopping to the main actor; keep the value type and initialization
the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3c710bb-724b-479d-ab18-48b5ac0d8e04
📒 Files selected for processing (3)
Sources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Workspace.swift
|
Follow-up push: Addressed the remaining minor review comments:
The remaining minor review threads are resolved. |
|
Fixed the CI/build break from the last review-follow-up patch. The sidebar now restores the parent-scope workspace lookup used for the precomputed terminal-scrollbar context-menu state, so stays on the precomputed-inputs path and compiles again. |
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 09b4bf0. Configure here.
|
Follow-up push: Addressed the remaining open review thread in
Validation:
|
…scrollbar-overlap Suppress right-edge scroll bar on TUI alt-screen and add disable setting (fixes 2728)
This branch was successfully deployed
1 participant
Fixes #2728
Root cause
nvim,htop,less,tmux,lazygit, andk9srepaint continuously, so AppKit keeps the overlay scroller effectively visible.What changed
total <= len), so the wrapper treats that as the alt-screen/no-scrollback signal and hides the bar for that surface.Show Terminal Scroll Bar, which defaults to on.Workspace Settings > Hide Terminal Scroll Bar.settings.jsonsupport and docs/schema forterminal.showScrollBar.Behavior note
nvimand other alt-screen TUIs, the scrollbar itself is hidden.Settings > Terminal > Show Terminal Scroll Baror per workspace viaWorkspace Settings > Hide Terminal Scroll Bar.Why the auto-hide from #2678 was not enough
How to use the new setting
Settings > Terminaland toggleShow Terminal Scroll Bar.Workspace Settings > Hide Terminal Scroll Bar.terminal.showScrollBarin~/.config/cmux/settings.json.Manual test checklist
nvimand confirm the rightmost cell column is unobstructed.htopand confirm the rightmost cell column is unobstructed.Show Terminal Scroll Barin Settings and confirm the bar and reserved gutter disappear immediately.Validation
./scripts/reload.sh --tag nvim-scrollbar-overlap.Note
Medium Risk
Changes terminal scroll bar visibility and sizing logic, plus adds new persisted settings and workspace state; risk is mainly UI/layout regressions and session restore compatibility issues.
Overview
Updates the embedded Ghostty terminal to only show the right-edge scroll bar when scrollback is available, suppressing it automatically for alternate-screen/TUI states (based on Ghostty scrollbar telemetry) and retiling/layouting when visibility changes.
Adds a global
Settings > Terminal > Show Terminal Scroll Bartoggle (backed by newTerminalScrollBarSettings,terminal.showScrollBarfile-managed setting, schema/docs updates, and localized strings) and a per-workspace context-menu override (Workspace Settings > Hide Terminal Scroll Bar) that is persisted in session snapshots and applied on restore.Reviewed by Cursor Bugbot for commit da1b45d. Bugbot is set up for automated code reviews on this repo. Configure here.