Widen font-size bounds + add Terminal Font Size setting#5338
Widen font-size bounds + add Terminal Font Size setting#5338lawrencecchen wants to merge 1 commit into
Conversation
Lets terminal/sidebar/tab-bar fonts go smaller and bigger: - Sidebar font range 10-20 -> 4-48, tab-bar font 8-14 -> 4-48. - New Terminal Font Size slider in Settings > Terminal, backed by the Ghostty `font-size` config key (default 12, range 4-72). Sets the base terminal font; the live Cmd +/- zoom remains handled by libghostty and is not bounded by this range. Localized en + ja. Updated the bounds-clamp tests to use inputs beyond the new ranges and added terminal font-size clamp coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces terminal font size as a configurable setting in the Ghostty terminal UI. It extends the existing font-size infrastructure with new config bounds, adds protocol methods to query and persist terminal font sizes, implements terminal font persistence in the settings view, and integrates search/localization support. ChangesTerminal Font Size Settings
Sequence DiagramsequenceDiagram
participant TerminalSection as TerminalSection<br/>(UI)
participant HostActions as HostSettingsActions<br/>(API)
participant GhosttyConfig as GhosttyConfig<br/>(Data Model)
participant CmuxEditor as CmuxGhosttyConfigSettingEditor<br/>(Config I/O)
TerminalSection->>HostActions: terminalFontSize()
HostActions->>GhosttyConfig: read fontSize + bounds constants
GhosttyConfig->>HostActions: fontSize + min/max/default
HostActions->>TerminalSection: SettingsFontSize object
TerminalSection->>TerminalSection: User adjusts slider
TerminalSection->>HostActions: setTerminalFontSize(points)
HostActions->>GhosttyConfig: clampedTerminalFontSize(value)
GhosttyConfig->>CmuxEditor: clampedTerminalFontSize(value)
CmuxEditor->>GhosttyConfig: clamped CGFloat
HostActions->>CmuxEditor: persistFontSize(terminalFontSizeKey, ...)
CmuxEditor->>CmuxEditor: write config + reload
HostActions->>TerminalSection: success (Bool)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40b57167d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let size = Double(value), size.isFinite { | ||
| fontSize = Self.clampedTerminalFontSize(CGFloat(size)) |
There was a problem hiding this comment.
When users already have a Ghostty font-size outside the new Settings slider range, this parse path now silently clamps their config on every load (for example font-size = 96 renders as 72), whereas the previous behavior preserved any finite value. The Settings setter already clamps values before writing, so the clamp should stay at the UI persistence boundary rather than changing how existing hand-authored Ghostty configs are interpreted.
Useful? React with 👍 / 👎.
| SettingsCardRow( | ||
| configurationReview: .settingsOnly, | ||
| String(localized: "settings.terminal.fontSize", defaultValue: "Terminal Font Size"), |
There was a problem hiding this comment.
The shipped Settings window builds its sidebar search from CmuxSettingsUI.SettingsSearchIndex/CuratedSettingEntry+Default, not the legacy Sources/SettingsNavigation table updated in this commit; because this new .settingsOnly row also has no explicit searchAnchorID, searching for terminal font size cannot surface or scroll/highlight this row in the active settings UI. Add the entry to the package search index and give this settings-only row the matching anchor so the new setting is discoverable.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 40b5716. Configure here.
| if let size = Double(value) { | ||
| fontSize = CGFloat(size) | ||
| if let size = Double(value), size.isFinite { | ||
| fontSize = Self.clampedTerminalFontSize(CGFloat(size)) |
There was a problem hiding this comment.
Clamped parse desyncs terminal UI
Low Severity
Parsing font-size now clamps into GhosttyConfig.fontSize, while libghostty still applies the raw config value on reload. The new terminal font slider and TextBox sizing read the clamped Swift value, so they can disagree with the live terminal when font-size is outside 4–72 pt.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 40b5716. Configure here.
Greptile SummaryWidens the sidebar (10–20 → 4–48) and tab-bar (8–14 → 4–48) font-size bounds, and introduces a new Terminal Font Size setting backed by Ghostty's
Confidence Score: 4/5Safe to merge. The new Terminal Font Size control is a clean addition that follows established patterns, and the wider sidebar/tab-bar bounds are a straightforward constant update backed by updated tests. The implementation is consistent with the existing sidebar and tab-bar font-size features. The only gap is that the five new user-facing strings land without translations for 19 of the 21 locales already in the catalog, which matches the existing pattern for similar strings but still leaves new Settings copy untranslated for most non-en/ja users. Resources/Localizable.xcstrings — new Terminal Font Size strings are present in en and ja only. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User drags slider / taps Reset\nTerminalSection.swift] --> B[saveTerminalFontSize points]
B --> C{terminalFontSaveTask?\ncancel previous}
C --> D[Task: await setTerminalFontSize]
D --> E[HostSettingsActions\nclampedTerminalFontSize\n4–72]
E --> F[persistFontSize key=font-size\nserial fontConfigWriter actor]
F --> G{Write succeeded?}
G -->|yes| H[Reload Ghostty config\npost ghosttyConfigDidReload]
G -->|no| I[terminalFontSaveFailed = true\nshow error row]
H --> J[GhosttyConfig.load\nparses font-size\nclamped to 4–72]
J --> K[Settings slider shows\nnew clamped value]
J --> L[TerminalPanelView.terminalFontSize\nupdated for TextBox font]
Reviews (1): Last reviewed commit: "Widen font-size bounds + add Terminal Fo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| } | ||
| }, | ||
| "settings.search.alias.setting.terminal.font-size": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "font-size terminal font size text scale shell zoom bigger smaller larger point base" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "font-size terminal font size ターミナル フォントサイズ テキスト 拡大 縮小 ズーム 大きく 小さく ポイント" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.search.alias.setting.terminal.tab-bar-font-size": { | ||
| "extractionState": "manual", | ||
| "localizations": { |
There was a problem hiding this comment.
The catalog contains 21 locales (ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, ok, pl, pt, pt-BR, px, ru, th, tr, uk), but all five new keys (settings.terminal.fontSize, .subtitle, .reset, .saveFailed, and settings.search.alias.setting.terminal.font-size) only carry en and ja entries. Any user running cmux in one of the other 19 locales will fall back to the English default at runtime, making the new Terminal Font Size setting and its error copy untranslated. This matches the pattern of the adjacent tab-bar font strings, but it still means new user-facing copy is landing without coverage across the supported locales.
Rule Used: Flag production user-facing text that is not fully... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/SettingsNavigation.swift`:
- Line 348: The settings registration adds a "font-size" settings path for the
terminal but SettingsSearchIndex.anchorID(forSettingsPath:) lacks a "font-size"
case, so deep-linking fails; update
SettingsSearchIndex.anchorID(forSettingsPath:) to include a mapping for the
"font-size" key (matching the terminal setting registered in SettingsNavigation,
e.g., the .terminal font-size entry) and return the same anchor identifier used
for the terminal font-size setting so config-path navigation/hightlight resolves
correctly; locate the anchorID switch/case (or dictionary) and add the
"font-size" branch returning the terminal font-size anchor.
🪄 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: 4cb6d242-07d2-4153-8740-e413b436230a
📒 Files selected for processing (10)
Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/TerminalSection.swiftResources/Localizable.xcstringsSources/CmuxApplicationSupportDirectories.swiftSources/GhosttyConfig.swiftSources/HostSettingsActions.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftcmuxTests/GhosttyConfigTests.swiftcmuxTests/SidebarOrderingTests.swift
| setting(.app, "palette-search-all", String(localized: "settings.app.commandPaletteSearchAllSurfaces", defaultValue: "Command Palette Searches All Surfaces"), "cmd p search terminal browser markdown"), | ||
| setting(.terminal, "scrollbar", String(localized: "settings.terminal.scrollBar", defaultValue: "Show Terminal Scroll Bar"), "terminal shell scrollback"), | ||
| setting(.terminal, "copy-on-select", String(localized: "settings.terminal.copyOnSelect", defaultValue: "Copy on Selection"), "terminal.copyOnSelect clipboard selection mouse double click triple click"), | ||
| setting(.terminal, "font-size", String(localized: "settings.terminal.fontSize", defaultValue: "Terminal Font Size"), "font size text scale terminal shell zoom bigger smaller font-size point"), |
There was a problem hiding this comment.
Add missing font-size settings-path anchor mapping for terminal font size.
Line 348 registers the searchable setting, but SettingsSearchIndex.anchorID(forSettingsPath:) has no "font-size" entry. Since terminal font persistence uses the Ghostty font-size key (Sources/HostSettingsActions.swift:209-215), deep-link/highlight flows from config-path navigation can fail to resolve this setting.
🔧 Proposed fix
private static let settingsPathAnchorIDs: [String: String] = [
+ "font-size": settingID(for: .terminal, idSuffix: "font-size"),
"rightSidebar.beta.feed.enabled": settingID(for: .betaFeatures, idSuffix: "feed"),
"rightSidebar.beta.dock.enabled": settingID(for: .betaFeatures, idSuffix: "dock"),🤖 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/SettingsNavigation.swift` at line 348, The settings registration adds
a "font-size" settings path for the terminal but
SettingsSearchIndex.anchorID(forSettingsPath:) lacks a "font-size" case, so
deep-linking fails; update SettingsSearchIndex.anchorID(forSettingsPath:) to
include a mapping for the "font-size" key (matching the terminal setting
registered in SettingsNavigation, e.g., the .terminal font-size entry) and
return the same anchor identifier used for the terminal font-size setting so
config-path navigation/hightlight resolves correctly; locate the anchorID
switch/case (or dictionary) and add the "font-size" branch returning the
terminal font-size anchor.
![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/GhosttyConfig.swift">
<violation number="1" location="Sources/GhosttyConfig.swift:408">
P2: Parse-time clamping of `font-size` can desync the Settings UI from the live terminal. libghostty applies the raw config value, but this path now clamps into the Swift model. For hand-authored configs with values outside 4–72 (e.g. `font-size = 96`), the slider will show 72 while the terminal actually renders at 96. Since `setTerminalFontSize` already clamps before writing, the clamp here is redundant and silently misrepresents the user's actual config. Consider preserving any finite value at parse time (matching the previous behavior) and only enforcing the range on the Settings write path.</violation>
</file>
<file name="Sources/SettingsNavigation.swift">
<violation number="1" location="Sources/SettingsNavigation.swift:348">
P2: Missing `settingsPathAnchorIDs` entry for `"font-size"`. The search index now includes this setting, but `anchorID(forSettingsPath:)` has no mapping for it, so config-path deep-link/highlight navigation will fail to resolve the terminal font size row.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if let size = Double(value) { | ||
| fontSize = CGFloat(size) | ||
| if let size = Double(value), size.isFinite { | ||
| fontSize = Self.clampedTerminalFontSize(CGFloat(size)) |
There was a problem hiding this comment.
P2: Parse-time clamping of font-size can desync the Settings UI from the live terminal. libghostty applies the raw config value, but this path now clamps into the Swift model. For hand-authored configs with values outside 4–72 (e.g. font-size = 96), the slider will show 72 while the terminal actually renders at 96. Since setTerminalFontSize already clamps before writing, the clamp here is redundant and silently misrepresents the user's actual config. Consider preserving any finite value at parse time (matching the previous behavior) and only enforcing the range on the Settings write path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyConfig.swift, line 408:
<comment>Parse-time clamping of `font-size` can desync the Settings UI from the live terminal. libghostty applies the raw config value, but this path now clamps into the Swift model. For hand-authored configs with values outside 4–72 (e.g. `font-size = 96`), the slider will show 72 while the terminal actually renders at 96. Since `setTerminalFontSize` already clamps before writing, the clamp here is redundant and silently misrepresents the user's actual config. Consider preserving any finite value at parse time (matching the previous behavior) and only enforcing the range on the Settings write path.</comment>
<file context>
@@ -401,8 +404,8 @@ struct GhosttyConfig {
- if let size = Double(value) {
- fontSize = CGFloat(size)
+ if let size = Double(value), size.isFinite {
+ fontSize = Self.clampedTerminalFontSize(CGFloat(size))
}
case "surface-tab-bar-font-size":
</file context>
| fontSize = Self.clampedTerminalFontSize(CGFloat(size)) | |
| fontSize = CGFloat(size) |
| setting(.app, "palette-search-all", String(localized: "settings.app.commandPaletteSearchAllSurfaces", defaultValue: "Command Palette Searches All Surfaces"), "cmd p search terminal browser markdown"), | ||
| setting(.terminal, "scrollbar", String(localized: "settings.terminal.scrollBar", defaultValue: "Show Terminal Scroll Bar"), "terminal shell scrollback"), | ||
| setting(.terminal, "copy-on-select", String(localized: "settings.terminal.copyOnSelect", defaultValue: "Copy on Selection"), "terminal.copyOnSelect clipboard selection mouse double click triple click"), | ||
| setting(.terminal, "font-size", String(localized: "settings.terminal.fontSize", defaultValue: "Terminal Font Size"), "font size text scale terminal shell zoom bigger smaller font-size point"), |
There was a problem hiding this comment.
P2: Missing settingsPathAnchorIDs entry for "font-size". The search index now includes this setting, but anchorID(forSettingsPath:) has no mapping for it, so config-path deep-link/highlight navigation will fail to resolve the terminal font size row.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/SettingsNavigation.swift, line 348:
<comment>Missing `settingsPathAnchorIDs` entry for `"font-size"`. The search index now includes this setting, but `anchorID(forSettingsPath:)` has no mapping for it, so config-path deep-link/highlight navigation will fail to resolve the terminal font size row.</comment>
<file context>
@@ -345,6 +345,7 @@ enum SettingsSearchIndex {
setting(.app, "palette-search-all", String(localized: "settings.app.commandPaletteSearchAllSurfaces", defaultValue: "Command Palette Searches All Surfaces"), "cmd p search terminal browser markdown"),
setting(.terminal, "scrollbar", String(localized: "settings.terminal.scrollBar", defaultValue: "Show Terminal Scroll Bar"), "terminal shell scrollback"),
setting(.terminal, "copy-on-select", String(localized: "settings.terminal.copyOnSelect", defaultValue: "Copy on Selection"), "terminal.copyOnSelect clipboard selection mouse double click triple click"),
+ setting(.terminal, "font-size", String(localized: "settings.terminal.fontSize", defaultValue: "Terminal Font Size"), "font size text scale terminal shell zoom bigger smaller font-size point"),
setting(.terminal, "tab-bar-font-size", String(localized: "settings.terminal.tabBarFontSize", defaultValue: "Tab Bar Font Size"), "font size text scale terminal browser pane tab title surface-tab-bar-font-size"),
setting(.terminal, "agent-auto-resume", String(localized: "settings.terminal.agentAutoResume", defaultValue: "Resume Agent Sessions on Reopen"), "terminal.autoResumeAgentSessions auto resume restore reopen relaunch quit sessions agents claude code codex opencode rovo dev rovodev toggle"),
</file context>
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
CodeRabbit now posts non-blocking comment reviews (request_changes_workflow=false, #5538).
1 participant
Lets the terminal, sidebar, and tab-bar fonts go smaller and bigger, and adds a Settings control for the terminal font.
font-sizekey (default 12, range 4–72). It sets the base terminal font; the live Cmd +/− zoom stays handled by libghostty and is not bounded by this range.Localized en + ja. Bounds-clamp tests updated to use inputs beyond the new ranges, plus new terminal font-size clamp tests. No new files (existing Section/host-action wiring reused).
This is phase 1 of the request (wider ranges). Two follow-ups are pending a product decision: per-font configurable min/max limits, and a global UI/app font scale (a larger cross-cutting change).
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
UI and Ghostty config persistence for font sizes; no auth or security-sensitive paths, with existing clamp/reload patterns reused.
Overview
Widens sidebar and tab-bar font sliders (4–48 pt) and adds a Terminal Font Size control in Settings → Terminal that persists Ghostty
font-size(default 12, 4–72 pt) with live reload—the slider sets the base size; Cmd +/- zoom is unchanged.Parsing and
CmuxGhosttyConfigSettingEditornow clamp terminalfont-sizeon load; settings search/localization (en/ja) and tests were updated for the new bounds and terminal setting.Reviewed by Cursor Bugbot for commit 40b5716. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Expanded font-size controls and added a Terminal Font Size setting so users can scale terminal, sidebar, and tab titles more flexibly. The terminal control sets the base font via
Ghosttywhile Cmd +/- live zoom stays unchanged.font-size; default 12, range 4–72). Sets the base size; Cmd +/- zoom remains handled bylibghosttyand is not bounded.Written for commit 40b5716. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests