Add configurable sidebar workspace font size#4798
Merged
austinywang merged 30 commits intoJun 1, 2026
Merged
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 configurable ChangesSidebar and Tab Bar Font Sizing Feature
Build Script and CI Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (14 passed)
✨ 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 |
…omizing-sidebar-workspace-tab
Contributor
Greptile SummaryAdds configurable font sizes for the left sidebar (
Confidence Score: 4/5Safe to merge after correcting the surface-tab-bar-font-size range claim in configUsage(); all other paths are well-isolated and the font-scale threading is correct. The configUsage() help string advertises 8–24 pt for surface-tab-bar-font-size but the clamp ceiling is 14 pt. A user who reads the help, sets 20, and observes 14 in the reload message has no explanation. Everything else — the async refresh pipeline, FontConfigWriter actor, snapshot equatable guard, BOM handling, and localization — looks correct. CLI/CMUXCLI+Config.swift — configUsage() range for surface-tab-bar-font-size needs to read 8–14 pt, not 8–24 pt. Important Files Changed
Sequence DiagramsequenceDiagram
participant Slider as Settings Slider
participant Section as SidebarSection / TerminalSection
participant Host as HostSettingsActions
participant Writer as FontConfigWriter (actor)
participant Disk as Ghostty config file
participant App as GhosttyApp.reloadConfiguration
participant Store as SidebarTabItemSettingsStore
participant Provider as SidebarFontSizeProvider (Task.detached)
Slider->>Section: "editing = false"
Section->>Section: saveSidebarFontSize(points)
Section->>Host: await setSidebarFontSize(points)
Host->>Writer: await write(key:value:)
Writer->>Disk: ConfigSourceEnvironment.writeCmuxConfigSetting
Disk-->>Writer: ok / error
Writer-->>Host: Bool
Host->>App: reloadConfiguration(source:) [main actor]
App--)Store: ghosttyConfigDidReload notification
Store->>Provider: await sidebarFontSizeProvider()
Provider->>Provider: "Task.detached { GhosttyConfig.load().sidebarFontSize }"
Provider-->>Store: CGFloat
Store->>Store: "refreshSnapshot() → @Published snapshot"
Reviews (25): Last reviewed commit: "test: use in-range 14 for tab-bar font l..." | 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.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/GhosttyConfigTests.swift`:
- Around line 4296-4364: The test suite SidebarFontSizeConfigTests currently
uses XCTestCase and XCTAssert* assertions; change it to Swift Testing style by
replacing the XCTest import and class SidebarFontSizeConfigTests: XCTestCase
with a `@Suite-annotated` type (e.g., `@Suite` struct SidebarFontSizeConfigTests)
and convert each test method
(testDefaultSidebarFontSizeMatchesSidebarTitleBaseline,
testParseSidebarFontSizeIntegerValue, testParseSidebarFontSizeFractionalValue,
testParseSidebarFontSizeClampsBelowMinimum,
testParseSidebarFontSizeClampsAboveMaximum,
testParseSidebarFontSizeIgnoresInvalidAndNonFiniteValues,
testLoadUsesParsedSidebarFontSizeFromInjectedLoader) into `@Test` functions,
replacing XCTAssertEqual calls with Swift Testing assertions (use `#expect` for
value checks with appropriate tolerance or try `#require` where setup can throw)
and adjust the GhosttyConfig usage and load helper accordingly so the logic
remains identical but follows the repo policy of using `@Suite/`@Test and
`#expect/`#require instead of XCTestCase/XCTAssert.
In `@cmuxTests/SidebarOrderingTests.swift`:
- Around line 118-160: Replace the XCTestCase-based test suite with a Swift
Testing suite: remove import XCTest and the final class
SidebarTabItemFontScaleTests : XCTestCase, add import Testing and declare a
`@Suite` (e.g., struct SidebarTabItemFontScaleTests) containing `@Test` functions
mirroring the existing test names (testDefaultSidebarFontScaleIsUnitScale,
testSidebarFontScaleIsProportionalToDefaultSidebarSize,
testSidebarFontScaleClampsSmallSizes, testSidebarFontScaleClampsLargeSizes,
testSidebarFontScaleFallsBackToDefaultForNonFiniteValue) and call
SidebarTabItemFontScale.scale(for: ...) and assert expected values using Swift
Testing assertions (e.g., Assertions.assertEqual with accuracy), keeping
references to GhosttyConfig.defaultSidebarFontSize,
GhosttyConfig.minSidebarFontSize, and GhosttyConfig.maxSidebarFontSize.
In `@Sources/ContentView.swift`:
- Around line 9747-9760: Move the pure font-scale logic out of ContentView by
creating a new Swift file (e.g., Sources/Sidebar/SidebarTabItemFontScale.swift)
and relocate the SidebarTabItemFontScale enum (and optionally
SidebarFontSizeProvider if you want the provider alongside it) there; ensure
SidebarTabItemFontScale.scale(for:) remains unchanged and references
GhosttyConfig as before, update any imports if needed, and adjust call sites in
ContentView to reference the extracted type so tests (e.g.,
SidebarOrderingTests) and consumers continue to compile and run.
- Around line 14487-14491: The badge size is being clamped by
scaledAccessorySize so it never shrinks below 16pt; split the logic so the
unread badge can use an unclamped scaled value while the close-button hit target
keeps a minimum. Replace the single scaledAccessorySize with two values (e.g.,
badgeSize = 16 * fontScale and accessoryHitSize =
max(SidebarTrailingAccessoryWidthPolicy.closeButtonWidth, max(16, 16 *
fontScale)) or apply the max only when computing scaledCloseButtonWidth) and use
badgeSize for the unread badge frame and accessoryHitSize /
scaledCloseButtonWidth for the close-button sizing/hit target.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 208d20fa-c2a4-4810-ac95-ac4c6475e7d7
📒 Files selected for processing (7)
Sources/ContentView.swiftSources/GhosttyConfig.swiftSources/ShortcutHintPill.swiftSources/Sidebar/SidebarDirectoryText.swiftcmuxTests/GhosttyConfigTests.swiftcmuxTests/SidebarOrderingTests.swiftweb/app/[locale]/docs/configuration/page.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/cmuxApp.swift`:
- Around line 6300-6303: The user-facing Settings message assigns
sidebarFontSizeErrorMessage using error.localizedDescription which exposes raw
internal error text; change this to a generic recovery message (e.g., "Couldn't
save sidebar font size. Please try again.") and remove interpolation of
error.localizedDescription from the string assigned to
sidebarFontSizeErrorMessage; instead, log the full error details to an internal
logger (e.g., use your existing Logger/os_log or a function near where
sidebarFontSizeErrorMessage is set) so the raw error is recorded for debugging
while the UI only shows the sanitized message.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74a9bdbe-3d3c-40cf-817c-9a7fdb24d710
📒 Files selected for processing (8)
CLI/CMUXCLI+Config.swiftResources/Localizable.xcstringsSources/CmuxApplicationSupportDirectories.swiftSources/GhosttyConfig.swiftSources/Settings/ConfigSource.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/cmuxApp.swift
![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.
1 issue found across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
GhosttyConfig.parse ignored a BOM-prefixed first line, so a BOM-encoded sidebar-font-size/surface-tab-bar-font-size setting would be dropped at runtime (defaulting) even though the CLI/Settings editor reads + writes it correctly. Strip the leading BOM to match parsedSetting/parsedConfigEntry. Addresses Cursor Bugbot finding on PR #4798. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review All actionable findings are addressed as of d397be5 (CI is now green). Since the last review:
|
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyConfig.swift (1)
404-414: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider adding finite-number validation to
font-sizefor consistency.The updated
surface-tab-bar-font-sizeparsing (line 408) and newsidebar-font-sizeparsing (line 412) both validatesize.isFinitebefore clamping. The existingfont-sizeparsing (line 404) does not. Adding the same finite check tofont-sizewould prevent edge cases (e.g.,font-size = infin config) and keep all three font-size parsings consistent.Proposed consistency fix
case "font-size": - if let size = Double(value) { + if let size = Double(value), size.isFinite { fontSize = CGFloat(size) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/GhosttyConfig.swift` around lines 404 - 414, In the "font-size" case, add the same finite-number check used by the other cases and clamp before assigning: when parsing "font-size" (the branch that sets fontSize), validate with if let size = Double(value), size.isFinite then assign via the existing clamping helper (e.g. fontSize = Self.clampedFontSize(CGFloat(size))) instead of directly doing fontSize = CGFloat(size) so it matches the "surface-tab-bar-font-size" and "sidebar-font-size" behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/SidebarSection.swift`:
- Line 104: Replace the use of String(format:) with a locale-aware approach:
either call String.localizedStringWithFormat using the localized key (currently
passed as String(localized: "settings.fontSize.valuePoints", defaultValue: "%@
pt")) and hostActions.formattedFontSize(sidebarFont.points) as arguments, or use
the modern String(localized:defaultValue:) with direct interpolation of
hostActions.formattedFontSize(sidebarFont.points); update the Text initializer
in SidebarSection (the line constructing the font size display) to use one of
these patterns so formatting is locale-aware.
In `@scripts/build-ghostty-cli-helper.sh`:
- Around line 61-68: The host-arch detection block duplicates the Rosetta-aware
logic; replace it by invoking the existing helper detected_host_arch() and
assign its result to the local host_arch instead of repeating the case/sysctl
logic. Update the snippet that currently defines local host_arch and runs the
case/sysctl checks to simply call detected_host_arch() (ensuring
detected_host_arch() is declared earlier in the script) and export/assign that
return value to host_arch so behavior remains identical but the logic is
centralized.
---
Outside diff comments:
In `@Sources/GhosttyConfig.swift`:
- Around line 404-414: In the "font-size" case, add the same finite-number check
used by the other cases and clamp before assigning: when parsing "font-size"
(the branch that sets fontSize), validate with if let size = Double(value),
size.isFinite then assign via the existing clamping helper (e.g. fontSize =
Self.clampedFontSize(CGFloat(size))) instead of directly doing fontSize =
CGFloat(size) so it matches the "surface-tab-bar-font-size" and
"sidebar-font-size" behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d6ccb919-54ed-4c0a-b802-3ef2034674ed
📒 Files selected for processing (23)
.github/workflows/ci.ymlCLI/CMUXCLI+Config.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsFontSize.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Scene/SettingsWindowScene.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/SidebarSection.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/TerminalSection.swiftResources/Localizable.xcstringsSources/CmuxApplicationSupportDirectories.swiftSources/ContentView.swiftSources/GhosttyConfig.swiftSources/HostSettingsActions.swiftSources/Settings/ConfigSource.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/ShortcutHintPill.swiftSources/Sidebar/SidebarAppearanceSupport.swiftSources/Sidebar/SidebarDirectoryText.swiftcmuxTests/GhosttyConfigTests.swiftcmuxTests/SidebarOrderingTests.swiftdocs/cli-contract.mdscripts/build-ghostty-cli-helper.shweb/app/[locale]/docs/configuration/page.tsx
- select_zig_for_target now calls detected_host_arch() instead of an inline copy of the Rosetta-aware host-arch logic (single source). - font-size value label uses modern localized interpolation String(localized:defaultValue:) instead of String(format:). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The String(localized:defaultValue:"\(value) pt") interpolation could render the literal "%@ pt" from the catalog instead of substituting the value. Use String.localizedStringWithFormat with the localized "%@ pt" format so the point size is always substituted (locale-aware). Addresses Cursor Bugbot finding on PR #4798. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bumps bonsplit to ddb46fe (manaflow-ai/bonsplit#140): the per-tab leading icon, close/pin glyphs, and the trailing split/new-terminal/new-browser control icons now scale with surface-tab-bar-font-size, so the tab bar icons grow with the text instead of staying fixed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Lower the surface-tab-bar-font-size maximum from 24 to 14 so the tab bar text and icons stay within the bar chrome. Updates the single source of truth (CmuxGhosttyConfigSettingEditor.maxSurfaceTabBarFontSize) and the package preview fallback; clamping, the slider range, and tests all derive from it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Slider release / Reset now route through saveSidebar/SurfaceTabBarFontSize, which cancels any pending save Task before starting the next one. Combined with the serial FontConfigWriter actor on the host side, rapid adjustments no longer leave a stale value reflected in the UI, and the save tasks are structured rather than fire-and-forget. Addresses cubic/Cursor concurrency feedback on PR #4798. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
loadUsesParsedSurfaceTabBarFontSizeFromInjectedLoader parsed surface-tab-bar-font-size = 16 and expected 16, but with the new 14pt maximum that value clamps to 14, so the test would fail. Use 14 (the max) so it exercises a valid in-range value. Addresses Cursor Bugbot finding on PR #4798. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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 OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 04f275f. Configure here.
Contributor
Author
|
@coderabbitai review All inline review threads are now resolved as of 04f275f:
Please re-review so the prior changes-requested can be cleared. |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Sources/CmuxApplicationSupportDirectories.swift (1)
300-313:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrim newlines in the config-line parser.
CRLF files leave a trailing
\ron each line after the"\n"split, and this helper currently preserves it insetting.value. That makesparsedFontSizefail to parse otherwise valid existing settings, so Windows-authored configs fall back to defaults until rewritten.Proposed fix
private static func parsedSetting(in line: String) -> (key: String, value: String)? { - var trimmed = line.trimmingCharacters(in: .whitespaces) + var trimmed = line.trimmingCharacters(in: .whitespacesAndNewlines) // Strip a leading UTF-8 BOM so a BOM-encoded first line still matches its // key (otherwise the setting reads as absent and a duplicate is appended). if trimmed.hasPrefix("\u{FEFF}") { trimmed.removeFirst() - trimmed = trimmed.trimmingCharacters(in: .whitespaces) + trimmed = trimmed.trimmingCharacters(in: .whitespacesAndNewlines) } guard !trimmed.isEmpty, !trimmed.hasPrefix("#"), let separator = trimmed.firstIndex(of: "=") else { return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/CmuxApplicationSupportDirectories.swift` around lines 300 - 313, The parsedSetting helper preserves trailing CR characters because it only trims .whitespaces; update parsedSetting to trim .whitespacesAndNewlines (for the initial trimmed var, the re-trim after BOM removal, and when computing key and value) so CRLF-terminated lines lose trailing "\r" and parsedFontSize etc. can parse Windows-authored config values correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/cli-contract.md`:
- Around line 336-342: Update the CLI contract docs to state the default and
valid range for the font-size commands: document that sidebar-font-size defaults
to 12.5 pt and is clamped to 10–20 pt, and apply the same note for
surface-tab-bar-font-size (or indicate if different); add this as an inline note
in the contract table rows for `config get/set sidebar-font-size` and `config
get/set surface-tab-bar-font-size` or as a short paragraph immediately after the
block listing those commands so users see the default (12.5 pt) and valid range
(10–20 pt) when using `config sidebar-font-size` and `config
surface-tab-bar-font-size`.
In `@Resources/Localizable.xcstrings`:
- Around line 104895-104911: The three identical format strings should be
consolidated: remove the duplicate entries
settings.sidebarAppearance.fontSize.points and
settings.terminal.tabBarFontSize.points from Localizable.xcstrings and update
the code that currently looks up those keys to reference the shared key
settings.fontSize.valuePoints instead; ensure settings.fontSize.valuePoints
contains the required localizations (en/ja) and update any UI/lookup calls
(e.g., wherever sidebar or terminal tab bar font size formatting is performed)
to use the single key so future changes only need one translation to be updated.
---
Duplicate comments:
In `@Sources/CmuxApplicationSupportDirectories.swift`:
- Around line 300-313: The parsedSetting helper preserves trailing CR characters
because it only trims .whitespaces; update parsedSetting to trim
.whitespacesAndNewlines (for the initial trimmed var, the re-trim after BOM
removal, and when computing key and value) so CRLF-terminated lines lose
trailing "\r" and parsedFontSize etc. can parse Windows-authored config values
correctly.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f0b49d1-bbe1-40a3-b074-5cd20520f5ef
📒 Files selected for processing (24)
.github/workflows/ci.ymlCLI/CMUXCLI+Config.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsFontSize.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Scene/SettingsWindowScene.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/SidebarSection.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/TerminalSection.swiftResources/Localizable.xcstringsSources/CmuxApplicationSupportDirectories.swiftSources/ContentView.swiftSources/GhosttyConfig.swiftSources/HostSettingsActions.swiftSources/Settings/ConfigSource.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/ShortcutHintPill.swiftSources/Sidebar/SidebarAppearanceSupport.swiftSources/Sidebar/SidebarDirectoryText.swiftcmuxTests/GhosttyConfigTests.swiftcmuxTests/SidebarOrderingTests.swiftdocs/cli-contract.mdscripts/build-ghostty-cli-helper.shvendor/bonsplitweb/app/[locale]/docs/configuration/page.tsx
Comment on lines
+336
to
+342
| | `config get sidebar-font-size` | Print the effective sidebar text size. | | ||
| | `config set sidebar-font-size <points>` | Write the sidebar text size to cmux's editable Ghostty config and reload the running app when available. | | ||
| | `config sidebar-font-size [points]` | Get the sidebar text size, or set it when a point size is provided. | | ||
| | `config get surface-tab-bar-font-size` | Print the effective workspace tab bar text size. | | ||
| | `config set surface-tab-bar-font-size <points>` | Write the workspace tab bar text size to cmux's editable Ghostty config and reload the running app when available. | | ||
| | `config surface-tab-bar-font-size [points]` | Get the workspace tab bar text size, or set it when a point size is provided. | | ||
| | `config get <key>`, `config set <key> <points>` | Generic get/set for `sidebar-font-size` and `surface-tab-bar-font-size`. | |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Document default values and valid ranges for font-size commands.
The new font-size config commands are documented, but users would benefit from knowing the default values and valid ranges. According to the PR objectives, sidebar-font-size defaults to 12.5 pt and is clamped to 10–20 pt.
Consider adding a note about defaults and valid ranges, either inline in the Contract column or in a new paragraph below line 342.
📝 Suggested documentation enhancement
Add after line 342:
| `config get <key>`, `config set <key> <points>` | Generic get/set for `sidebar-font-size` and `surface-tab-bar-font-size`. |
+
+Font size values are clamped to the range 10.0–20.0 points. The default `sidebar-font-size` is 12.5 pt.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `config get sidebar-font-size` | Print the effective sidebar text size. | | |
| | `config set sidebar-font-size <points>` | Write the sidebar text size to cmux's editable Ghostty config and reload the running app when available. | | |
| | `config sidebar-font-size [points]` | Get the sidebar text size, or set it when a point size is provided. | | |
| | `config get surface-tab-bar-font-size` | Print the effective workspace tab bar text size. | | |
| | `config set surface-tab-bar-font-size <points>` | Write the workspace tab bar text size to cmux's editable Ghostty config and reload the running app when available. | | |
| | `config surface-tab-bar-font-size [points]` | Get the workspace tab bar text size, or set it when a point size is provided. | | |
| | `config get <key>`, `config set <key> <points>` | Generic get/set for `sidebar-font-size` and `surface-tab-bar-font-size`. | | |
| | `config get sidebar-font-size` | Print the effective sidebar text size. | | |
| | `config set sidebar-font-size <points>` | Write the sidebar text size to cmux's editable Ghostty config and reload the running app when available. | | |
| | `config sidebar-font-size [points]` | Get the sidebar text size, or set it when a point size is provided. | | |
| | `config get surface-tab-bar-font-size` | Print the effective workspace tab bar text size. | | |
| | `config set surface-tab-bar-font-size <points>` | Write the workspace tab bar text size to cmux's editable Ghostty config and reload the running app when available. | | |
| | `config surface-tab-bar-font-size [points]` | Get the workspace tab bar text size, or set it when a point size is provided. | | |
| | `config get <key>`, `config set <key> <points>` | Generic get/set for `sidebar-font-size` and `surface-tab-bar-font-size`. | | |
| Font size values are clamped to the range 10.0–20.0 points. The default `sidebar-font-size` is 12.5 pt. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/cli-contract.md` around lines 336 - 342, Update the CLI contract docs to
state the default and valid range for the font-size commands: document that
sidebar-font-size defaults to 12.5 pt and is clamped to 10–20 pt, and apply the
same note for surface-tab-bar-font-size (or indicate if different); add this as
an inline note in the contract table rows for `config get/set sidebar-font-size`
and `config get/set surface-tab-bar-font-size` or as a short paragraph
immediately after the block listing those commands so users see the default
(12.5 pt) and valid range (10–20 pt) when using `config sidebar-font-size` and
`config surface-tab-bar-font-size`.
Comment on lines
+104895
to
+104911
| "settings.fontSize.valuePoints": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "%@ pt" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "%@ pt" | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider consolidating duplicate format strings.
The format string %@ pt is defined three times:
settings.fontSize.valuePoints(line 104895)settings.sidebarAppearance.fontSize.points(line 138679)settings.terminal.tabBarFontSize.points(line 142719)
Since settings.fontSize.valuePoints appears to be the shared/generic key, the code consuming these strings could reference that single key instead of duplicating it. This would simplify future maintenance if the format ever needs to change.
Also applies to: 138679-138695, 142719-142735
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Resources/Localizable.xcstrings` around lines 104895 - 104911, The three
identical format strings should be consolidated: remove the duplicate entries
settings.sidebarAppearance.fontSize.points and
settings.terminal.tabBarFontSize.points from Localizable.xcstrings and update
the code that currently looks up those keys to reference the shared key
settings.fontSize.valuePoints instead; ensure settings.fontSize.valuePoints
contains the required localizations (en/ja) and update any UI/lookup calls
(e.g., wherever sidebar or terminal tab bar font size formatting is performed)
to use the single key so future changes only need one translation to be updated.
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sidebar-font-sizeto Ghostty config with a 12.5pt default and 10-20pt clamp.SidebarTabItemSettingsSnapshotsoTabItemView.equatable()continues to protect the typing-sensitive sidebar row path.Closes #2643.
Architecture
The config file remains the source of truth.
SidebarTabItemSettingsStoreis the single MainActor owner that observes user-default and Ghostty config reload notifications, computes an immutableSidebarTabItemSettingsSnapshot, and passes only value data below the sidebar list boundary. No new store references, body-time config reads, or timing repair hooks were added underTabItemView.Tests
Not run locally per task instruction. Added runtime behavior coverage in already-wired test files:
SidebarFontSizeConfigTestsfor default, parse, fractional value, clamping, invalid/non-finite input, and injected load path.SidebarTabItemSettingsSnapshotFontScaleTestsfor default scale, proportional scale, clamping, and snapshot equality changes.HQ build command
CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-2643-allow-customizing-sidebar-workspace-tab --launchNeed help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches Ghostty config persistence, live reload, and a large sidebar rendering path in ContentView; behavior is bounded by clamps and tests but regressions could affect layout or typing-sensitive sidebar performance.
Overview
Adds Ghostty-backed font sizing for the left sidebar (
sidebar-font-size, 10–20 pt, default 12.5) and workspace tab bar (surface-tab-bar-font-size, clamped in app), with sharedCmuxGhosttyConfigSettingEditorread/write/parse logic (BOM-safe, symlink-aware).Settings get sliders in Sidebar and Terminal sections via new
SettingsFontSize/SettingsHostActions; the host persists off the main actor, reloads config, and shows save failures.Sidebar rows scale titles, metadata, badges, PR/port rows, shortcut hints, and close targets from a reload-aware
SidebarTabItemSettingsSnapshot/ store (no extra per-row config reads).CLI:
cmux config get|setand shorthand commands for both keys, optional live reload when a socket is available.Also updates docs/localization/search, unit tests for parse/clamp/editor round-trips, CI (
CMUX_SKIP_ZIG_BUILDon Release universal build, Mach-O Zig stub + Zig selection fixes), and a small web config example.Reviewed by Cursor Bugbot for commit 04f275f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests