Skip to content

Add sidebar-font-size Ghostty config option (closes #2643)#2990

Closed
livermush wants to merge 14 commits into
manaflow-ai:mainfrom
livermush:sidebar-font-scale
Closed

Add sidebar-font-size Ghostty config option (closes #2643)#2990
livermush wants to merge 14 commits into
manaflow-ai:mainfrom
livermush:sidebar-font-scale

Conversation

@livermush

@livermush livermush commented Apr 18, 2026

Copy link
Copy Markdown

Summary

  • Add sidebar-font-size Ghostty config key (default 12.5, clamped 10–20) to scale sidebar workspace tab labels and all proportional sidebar chrome.
  • All secondary sidebar text (subtitle, branch/ports, progress label, PR/remote rows, log entries, markdown metadata blocks) scales from the primary size via scale = sidebarFontSize / 12.5, preserving the existing visual hierarchy.
  • Exposed on SidebarTabItemSettingsSnapshot (already Equatable and observed by TabItemView) so the typing-latency contract on TabItemView is preserved — no new @ObservedObject or body-time lookups on the hot path.

Closes #2643.

Approach

Mirrors the existing font-size / surface-tab-bar-font-size pattern:

  1. GhosttyConfig gains sidebarFontSize: CGFloat with default=12.5, min=10, max=20, and a parser case for sidebar-font-size.
  2. SidebarTabItemSettingsSnapshot computes sidebarFontScale = sidebarFontSize / defaultSidebarFontSize and refreshes on .ghosttyConfigDidReload. The Equatable conformance already covers the new property via the full-snapshot equality check, so no existing callers need changes.
  3. Each .font(.system(size: N, ...)) inside the sidebar tree was replaced with N * fontScale where fontScale is read from the pre-computed snapshot. Scope limited to TabItemView and its nested views.

Scope

In scope (all in Sources/ContentView.swift):

  • TabItemView — title, unread dot, pin, close, subtitle, log icon + text, progress label, branch row, ports, PR status, remote
  • PullRequestStatusIcon
  • SidebarWorkspaceDescriptionText
  • SidebarMetadataRows + SidebarMetadataEntryRow
  • SidebarMetadataMarkdownBlocks + SidebarMetadataMarkdownBlockRow

Intentionally out of scope (can be follow-ups):

Usage

# ~/.config/ghostty/config  (or cmux's Ghostty config path)
sidebar-font-size = 14

Then either restart cmux or run cmux reload-config — running windows pick the new size up via the existing .ghosttyConfigDidReload path.

Values outside 10–20 are clamped. Missing / non-numeric values fall back to the 12.5 default.

Test plan

  • xcodebuild -scheme cmux-unit build-for-testing — compiles clean
  • ./scripts/reload.sh --tag sidebar-font-scale — Debug build succeeds
  • Added SidebarFontSizeConfigTests (default, valid parse, fractional, min+max clamp, non-numeric fallback)
  • Manual: sidebar-font-size = 10 — sidebar renders at min, no text clipping
  • Manual: sidebar-font-size = 12.5 — no visual diff vs. main
  • Manual: sidebar-font-size = 15 — sidebar text scales up, layout balanced
  • Manual: sidebar-font-size = 20 — sidebar renders at max, row heights expand without overlap
  • Manual: live-reload via cmux reload-config — open windows pick up new value
  • Manual: typing in a terminal surface — no regression in typing latency with sidebar expanded

Notes for reviewers

  • TabItemView is latency-sensitive (per CLAUDE.md: "do not add @EnvironmentObject, @ObservedObject (besides tab), or @Binding properties without updating the == function"). The scale is folded into the existing SidebarTabItemSettingsSnapshot — already Equatable, already covered by ==, already listened to via a single NotificationCenter observer in SidebarTabItemSettingsStore. No new observers on the hot path.
  • scale is a CGFloat multiplication at the call site — no per-frame allocation, no per-frame lookup.
  • Two-commit structure: feat commit + test commit (tests added second to keep implementation legible in isolation; this is feature work not a regression per CLAUDE.md's two-commit policy).

Summary by cubic

Adds a sidebar-font-size Ghostty config to scale all sidebar text and chrome (10–20 pt, default 12.5) with live reload. Closes #2643.

  • New Features

    • Adds sidebar-font-size (clamped) and derives sidebarFontScale on SidebarTabItemSettingsSnapshot; store refreshes on .ghosttyConfigDidReload.
    • Applies scale across TabItemView and subviews (titles/subtitles, directory text, metadata rows/markdown blocks, remote/branch/ports, logs, PR rows/icons via transform, shortcut hint pill via the shared overlay) while preserving the Equatable snapshot path.
  • Bug Fixes

    • Reject non-finite values (nan, inf) during parse to prevent NaN scales.
    • Scale PR status icons and frames, unread badge, close button, and trailing accessory containers with fontScale (16 pt min) to avoid clipping.
    • Only reserve the trailing gutter when an accessory is shown; keep prior layout when none is present.

Written for commit 0241ddd. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Configurable sidebar font size (10–20 pt, default 12.5) that immediately scales sidebar typography, badges, icons, accessory sizing and various sidebar layout elements (including pull-request status visuals).
  • Bug Fixes
    • Sidebar now responds to config reloads and cleans up observers to ensure reliable live updates.
  • Tests
    • Added coverage for parsing, defaults, numeric edge cases, clamping, scaling outcomes and snapshot/equality semantics.

Review Change Stack

Doug Hairfield and others added 3 commits April 18, 2026 17:50
Adds a configurable sidebar font size (10-20pt, default 12.5) via the
Ghostty config key `sidebar-font-size`. All other sidebar text (close
buttons, metadata rows, progress labels, PR/remote/branch info, log
entries, markdown blocks) scales proportionally from this primary size,
preserving the existing visual hierarchy.

Scope: TabItemView and its sub-views (SidebarWorkspaceDescriptionText,
SidebarMetadataRows, SidebarMetadataEntryRow, SidebarMetadataMarkdownBlocks,
SidebarMetadataMarkdownBlockRow, PullRequestStatusIcon). Other sidebar
chrome (feedback composer, help menu, dev footer) out of scope for this
change.

The scale is exposed on SidebarTabItemSettingsSnapshot (already Equatable
and observed by TabItemView) so the typing-latency contract around
TabItemView equatable snapshotting is preserved — no new @ObservedObject
or body-time lookups in the hot path.

Refs manaflow-ai#2643

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds SidebarFontSizeConfigTests matching the SidebarBackgroundConfigTests
pattern: verify default, valid parse, fractional parse, min/max clamp,
and non-numeric fallback to default.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ctable

Adds:

- GhosttyConfigTests: trailing-zero parse (14.0 == 14), injected-loader
  round-trip through GhosttyConfig.load(loadFromDisk:), and later-path-
  wins semantics (the contract that ~/Library/App Support overrides
  ~/.config/ghostty when both are present).
- SidebarTabItemSettingsSnapshotFontScaleTests: unit scale at default,
  proportional scaling at 18pt (→ 1.44), clamp-up below min, clamp-down
  above max, Equatable-flips-on-scale-diff (protects the
  TabItemView.equatable() re-render contract), Equatable-holds-on-match.

Refactor to support the snapshot tests:

- SidebarTabItemSettingsSnapshot is promoted from private to internal so
  `@testable import cmux_DEV` can see it. The store and view both live
  in the same file, so no external API is newly exposed — only tests.
- init now takes an injectable `sidebarFontSizeProvider: () -> CGFloat`
  defaulting to `{ GhosttyConfig.load().sidebarFontSize }`. Production
  behavior is unchanged; tests pass a constant closure to avoid touching
  the real config file.

All 15 new tests pass locally via
`xcodebuild test-without-building -scheme cmux-unit \
  -only-testing:cmuxTests/SidebarFontSizeConfigTests \
  -only-testing:cmuxTests/SidebarTabItemSettingsSnapshotFontScaleTests`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel

vercel Bot commented Apr 18, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Apr 18, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds a new Ghostty config key sidebar-font-size, computes a derived sidebarFontScale (injectable) in the sidebar snapshot, refreshes settings on config reload, threads fontScale through TabItemView and many sidebar subviews (including PR icons), updates SidebarDirectoryText, and adds parsing and snapshot tests.

Changes

Sidebar font scaling + wiring

Layer / File(s) Summary
Config constants & field
Sources/GhosttyConfig.swift
Add defaultSidebarFontSize, minSidebarFontSize, maxSidebarFontSize and instance sidebarFontSize (defaulted).
Config parsing
Sources/GhosttyConfig.swift
Parse sidebar-font-size as a finite Double, clamp to min/max, ignore non-finite tokens, and assign to sidebarFontSize.
Snapshot data & API
Sources/ContentView.swift
SidebarTabItemSettingsSnapshot gains sidebarFontScale and an injectable sidebarFontSizeProvider in its initializer; helper double(defaults:key:defaultValue:) removed.
Settings store observer
Sources/ContentView.swift
SidebarTabItemSettingsStore subscribes to .ghosttyConfigDidReload (in addition to UserDefaults.didChangeNotification), stores the observer reference, calls refreshSnapshot() on reload, and removes the observer in deinit.
Tab row scaling entry point
Sources/ContentView.swift
TabItemView adds computed fontScale = settings.sidebarFontScale and centralizes scaled accessory sizing (scaledAccessorySize, close hit-target sizing).
Apply scale to row elements
Sources/ContentView.swift (TabItemView spans)
Apply fontScale to workspace title/subtitle, unread/pin badge sizing, remote subtitle/connection status, description text, log/progress labels, git branch/directory (vertical & compact), pull request rows, ports monospaced text, shortcut pill font, and close-accessory glyph sizing/hit target.
Pull request icon scaling
Sources/ContentView.swift (PullRequestStatusIcon)
PullRequestStatusIcon accepts fontScale; open/merged icons use .scaleEffect(fontScale) with proportional frames and expose intrinsicFrameSize; closed state uses a scaled SF Symbol font size and proportional frame.
Subview plumbing for scaled typography
Sources/ContentView.swift, Sources/Sidebar/SidebarDirectoryText.swift
Add fontScale parameters and multiply base font sizes by fontScale for description, metadata rows, markdown blocks, metadata entry icons/toggles, and directory monospaced text.

Tests

Layer / File(s) Summary
Unit tests: parsing
cmuxTests/GhosttyConfigTests.swift
Add SidebarFontSizeConfigTests: default value, integer/fractional parsing, clamping to min/max, ignore non-numeric/non-finite tokens, loader loadFromDisk behavior, and last-write-wins semantics.
Unit tests: snapshot
cmuxTests/SidebarOrderingTests.swift
Add SidebarTabItemSettingsSnapshotFontScaleTests using isolated UserDefaults(suiteName:) and injected sidebarFontSizeProvider to validate default scale, proportional scaling, clamping, and snapshot equality/inequality; tests clean up persistent domains in teardown.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ConfigLoader
  participant GhosttyConfig
  participant SidebarTabItemSettingsStore
  participant TabItemView
  participant SidebarSubViews

  User->>ConfigLoader: edit config (sidebar-font-size)
  ConfigLoader->>GhosttyConfig: parse(new config)
  GhosttyConfig-->>SidebarTabItemSettingsStore: post .ghosttyConfigDidReload
  SidebarTabItemSettingsStore->>SidebarTabItemSettingsStore: compute SidebarTabItemSettingsSnapshot (sidebarFontScale)
  SidebarTabItemSettingsStore-->>TabItemView: publish updated settings
  TabItemView->>SidebarSubViews: pass fontScale -> render scaled fonts/icons/layout
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

  • manaflow-ai/cmux#3730: Touches SidebarDirectoryText rendering and is related to sidebar path candidate layout and sizing.

Poem

"I nibble code and tweak each line,
A sidebar font now feels just fine.
Scales that stretch in gentle tune,
Tabs grow cozy by the moon.
— 🐰"


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Cmux Swift @Concurrent ❌ Error SidebarTabItemSettingsSnapshot's default sidebarFontSizeProvider calls GhosttyConfig.load() (file-I/O-heavy) from @MainActor without async hop, violating concurrency rules for UI isolation. Make GhosttyConfig.load() nonisolated async @concurrent, or pass an alternative provider avoiding disk access from @MainActor init.
Docstring Coverage ⚠️ Warning Docstring coverage is 3.45% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the primary change: adding a sidebar-font-size Ghostty config option and references the closed issue.
Linked Issues check ✅ Passed All coding requirements from issue #2643 are met: sidebarFontSize added to GhosttyConfig with parsing and clamping, sidebar text scaled via sidebarFontScale, secondary text scales proportionally, and live-reload via .ghosttyConfigDidReload is preserved.
Out of Scope Changes check ✅ Passed All changes are directly tied to the sidebar-font-size feature implementation; intentional out-of-scope items (surface tab bar, footer views) are documented and deferred to follow-ups.
Cmux Swift Actor Isolation ✅ Passed SidebarTabItemSettingsSnapshot is an immutable value type created only in @MainActor contexts; GhosttyConfig.load() uses NSLock-protected cache providing thread safety; no actor isolation violations.
Cmux Swift Blocking Runtime ✅ Passed The PR adds sidebar-font-size config without new blocking primitives. All new code uses property assignments and arithmetic operations only, avoiding semaphores, locks, sleeps, or asyncAfter calls.
Cmux No Hacky Sleeps ✅ Passed PR contains only Swift code. The check scope is TypeScript, JavaScript, shell, and non-Swift build/runtime scripts; Swift timing is covered separately.
Cmux Swift Concurrency ✅ Passed PR adds no new legacy async patterns. Task{@MainActor} usage is inside AppKit NotificationCenter callbacks (allowed). Combine pattern on SidebarTabItemSettingsStore appears pre-existing.
Cmux Swift File And Package Boundaries ✅ Passed Adds +132/-60 net lines to oversized ContentView, under 250-line threshold. Qualifies for Exception #2: small UI/Ghostty glue with no independent feature logic requiring package extraction.
Cmux Swift Logging ✅ Passed PR adds sidebar-font-size feature with no logging violations. GhosttyConfig parsing, snapshot init, and font scaling code contain no print/NSLog/debugPrint/dump. No new debug logging introduced.
Cmux User-Facing Error Privacy ✅ Passed PR adds sidebar-font-size config with no user-facing errors, alerts, or privacy violations. Silent error handling matches existing font-size pattern; no sensitive data exposed.
Cmux Full Internationalization ✅ Passed PR only adds numeric font-scaling logic and config parsing; no new user-facing text strings or String(localized:) calls were added, so internationalization check passes.
Cmux Swiftui State Layout ✅ Passed PR extends existing ObservableObject by adding ghosttyConfigObserver. All views receive fontScale as value parameters, not store references. No GeometryReader measurement or render-time mutations.
Cmux Architecture Rethink ✅ Passed Required platform bridge observer, snapshot Equatable preserves hot-path, fontScale derived, no timing/dispatch, proper cleanup, comprehensive tests.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR adds sidebar-font-size config and scales sidebar UI; no new windows/WindowGroups created. Pre-existing NSWindow refs are handlers; test fixture allowed per rule.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering all required template sections with clear detail on what changed, why, testing approach, and a checklist.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a sidebar-font-size Ghostty config key (default 12.5 pt, clamped 10–20) that proportionally scales all sidebar workspace label text and chrome. The implementation mirrors the existing font-size / surface-tab-bar-font-size pattern and folds the scale into the already-Equatable SidebarTabItemSettingsSnapshot, preserving the hot-path contract on TabItemView.

  • GhosttyConfig gains sidebarFontSize with static min/max/default constants, parsing that rejects non-finite values, and double-clamp safety; SidebarTabItemSettingsSnapshot derives sidebarFontScale via an injectable closure so tests can bypass disk I/O.
  • SidebarTabItemSettingsStore adds a ghosttyConfigObserver for .ghosttyConfigDidReload with proper deinit cleanup; TabItemView and all nested sidebar sub-views propagate fontScale as a plain CGFloat parameter.
  • PullRequestOpenIcon / PullRequestMergedIcon are scaled via .scaleEffect(fontScale) + intrinsicFrameSize * fontScale frame to avoid per-coordinate path rewrites.

Confidence Score: 5/5

Safe to merge — the change is additive, follows the existing font-size config pattern throughout, and all affected paths have test coverage.

The config parsing, snapshot derivation, live-reload wiring, and view-hierarchy propagation are all correct. Cache invalidation happens before the reload notification is posted, so the snapshot always reads fresh config. The Equatable path is synthesized and correctly includes sidebarFontScale, ensuring TabItemView re-renders on config change. No new blocking synchronization, actor isolation mistakes, or user-facing strings requiring localization were introduced.

No files require special attention.

Important Files Changed

Filename Overview
Sources/GhosttyConfig.swift Adds sidebarFontSize property with static min/max/default constants; parser rejects non-finite (NaN/Inf) values and clamps inline. Clean.
Sources/ContentView.swift Propagates fontScale from SidebarTabItemSettingsSnapshot across TabItemView hierarchy; adds ghosttyConfigObserver with proper deinit cleanup; PullRequestOpenIcon/Merged now use scaleEffect + sized frames. No stale-state or invalidation issues found.
Sources/ShortcutHintPill.swift Adds fontScale parameter (default 1.0) to the shortcutHintPill view extension; threads it into ShortcutHintPill's fontSize. Minimal, safe change.
Sources/Sidebar/SidebarDirectoryText.swift Adds fontScale parameter (default 1); scales all three Text font sizes. Consistent with call sites in ContentView.swift.
cmuxTests/GhosttyConfigTests.swift Adds SidebarFontSizeConfigTests covering default, valid parse, fractional, clamping, non-numeric fallback, NaN/Inf rejection, trailing zero, last-wins override, and the loadFromDisk injection path.
cmuxTests/SidebarOrderingTests.swift Adds SidebarTabItemSettingsSnapshotFontScaleTests verifying unit scale at default, proportional scale at 18, clamp-up/down, Equatable differentiates scales, and Equatable holds when scales match.

Sequence Diagram

sequenceDiagram
    participant Config as GhosttyConfig
    participant Store as SidebarTabItemSettingsStore
    participant Snapshot as SidebarTabItemSettingsSnapshot
    participant View as TabItemView

    Note over Config: User edits sidebar-font-size
    Config->>Config: invalidateLoadCache()
    Config->>Store: post(.ghosttyConfigDidReload)
    Store->>Store: ghosttyConfigObserver fires (queue: .main)
    Store->>Snapshot: init(sidebarFontSizeProvider)
    Snapshot->>Config: GhosttyConfig.load() cache miss
    Config-->>Snapshot: sidebarFontSize clamped 10-20
    Snapshot->>Snapshot: "sidebarFontScale = clamped / 12.5"
    Snapshot-->>Store: new snapshot
    Store->>View: "@Published triggers re-eval"
    Note over View: Synthesized Equatable includes sidebarFontScale
    View->>View: "Apply N * fontScale to all font sizes"
Loading

Reviews (7): Last reviewed commit: "Merge origin/main into sidebar-font-scal..." | Re-trigger Greptile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (2)
Sources/ContentView.swift (2)

14247-14340: ⚠️ Potential issue | 🟡 Minor

Scale the custom PR icons with the row text.

fontScale only reaches the closed SF Symbol; the custom open/merged icons keep fixed geometry, and the closed icon still uses a fixed frame. This makes PR rows inconsistent at non-default sidebar font sizes.

Proposed direction
 private struct PullRequestStatusIcon: View {
     let status: SidebarPullRequestStatus
     let color: Color
     var fontScale: CGFloat = 1
-    private static let frameSize: CGFloat = 12
+    private var frameSize: CGFloat { 13 * fontScale }

     var body: some View {
         switch status {
         case .open:
-            PullRequestOpenIcon(color: color)
+            PullRequestOpenIcon(color: color, fontScale: fontScale)
         case .merged:
-            PullRequestMergedIcon(color: color)
+            PullRequestMergedIcon(color: color, fontScale: fontScale)
         case .closed:
             Image(systemName: "xmark.circle")
                 .font(.system(size: 7 * fontScale, weight: .regular))
                 .foregroundColor(color)
-                .frame(width: Self.frameSize, height: Self.frameSize)
+                .frame(width: frameSize, height: frameSize)
         }
     }
 }

 private struct PullRequestOpenIcon: View {
     let color: Color
-    private static let stroke = StrokeStyle(lineWidth: 1.2, lineCap: .round, lineJoin: .round)
-    private static let nodeDiameter: CGFloat = 3.0
-    private static let frameSize: CGFloat = 13
+    var fontScale: CGFloat = 1
+    private var stroke: StrokeStyle {
+        StrokeStyle(lineWidth: 1.2 * fontScale, lineCap: .round, lineJoin: .round)
+    }
+    private var nodeDiameter: CGFloat { 3.0 * fontScale }
+    private var frameSize: CGFloat { 13 * fontScale }

     var body: some View {
         ZStack {
             Path { path in
-                path.move(to: CGPoint(x: 3.0, y: 4.8))
+                path.move(to: CGPoint(x: 3.0 * fontScale, y: 4.8 * fontScale))
                 // scale the remaining path coordinates the same way
             }
-            .stroke(color, style: Self.stroke)
+            .stroke(color, style: stroke)
             // scale node frames/positions similarly
         }
-        .frame(width: Self.frameSize, height: Self.frameSize)
+        .frame(width: frameSize, height: frameSize)
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 14247 - 14340, The custom PR icons
(PullRequestOpenIcon, PullRequestMergedIcon) and the closed Image use fixed
geometry so they don't scale with sidebar font size; update
PullRequestStatusIcon to pass the fontScale down and change PullRequestOpenIcon
and PullRequestMergedIcon to accept a fontScale: CGFloat (default 1) and
multiply their static dimensions (frameSize, nodeDiameter, positions,
StrokeStyle.lineWidth) by fontScale (or compute scaled constants) so the Path
coordinates, Circle positions, and .frame sizes scale, and change the
closed-case Image to use .frame(width: Self.frameSize * fontScale, height:
Self.frameSize * fontScale) and .font(.system(size: 7 * fontScale, ...)) so all
three branches (PullRequestOpenIcon, PullRequestMergedIcon, and closed Image)
scale consistently with the fontScale passed from PullRequestStatusIcon.

13151-13152: ⚠️ Potential issue | 🟡 Minor

Scale the workspace shortcut hint too.

The shortcut pill still uses a fixed 10pt font, so sidebar-font-size won’t affect this sidebar text.

Proposed fix
-                        ShortcutHintPill(text: workspaceShortcutLabel, fontSize: 10, emphasis: shortcutHintEmphasis)
+                        ShortcutHintPill(text: workspaceShortcutLabel, fontSize: 10 * fontScale, emphasis: shortcutHintEmphasis)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 13151 - 13152, The ShortcutHintPill
currently hardcodes fontSize: 10 which prevents it from honoring the sidebar
font size setting; update the call in the showsWorkspaceShortcutHint /
workspaceShortcutLabel block to pass the dynamic sidebar font size used
elsewhere (e.g., replace the literal 10 with the same variable/property that
other sidebar text uses, such as sidebarFontSize or the shared fontSize binding)
so ShortcutHintPill(text: workspaceShortcutLabel, fontSize: <sidebarFontSize>,
emphasis: shortcutHintEmphasis) scales with the sidebar-font-size setting.
🤖 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/GhosttyConfig.swift`:
- Around line 258-265: The parsing for "sidebar-font-size" in GhosttyConfig
allows NaN/infinite values to slip through; before clamping and assigning to
sidebarFontSize, validate the parsed Double (e.g. the local size variable) by
rejecting non-finite or NaN values (use size.isFinite or !size.isNaN) and only
perform the min/max clamp when the value is finite; otherwise fall back to
leaving sidebarFontSize unchanged (or the existing invalid-value behavior).

---

Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 14247-14340: The custom PR icons (PullRequestOpenIcon,
PullRequestMergedIcon) and the closed Image use fixed geometry so they don't
scale with sidebar font size; update PullRequestStatusIcon to pass the fontScale
down and change PullRequestOpenIcon and PullRequestMergedIcon to accept a
fontScale: CGFloat (default 1) and multiply their static dimensions (frameSize,
nodeDiameter, positions, StrokeStyle.lineWidth) by fontScale (or compute scaled
constants) so the Path coordinates, Circle positions, and .frame sizes scale,
and change the closed-case Image to use .frame(width: Self.frameSize *
fontScale, height: Self.frameSize * fontScale) and .font(.system(size: 7 *
fontScale, ...)) so all three branches (PullRequestOpenIcon,
PullRequestMergedIcon, and closed Image) scale consistently with the fontScale
passed from PullRequestStatusIcon.
- Around line 13151-13152: The ShortcutHintPill currently hardcodes fontSize: 10
which prevents it from honoring the sidebar font size setting; update the call
in the showsWorkspaceShortcutHint / workspaceShortcutLabel block to pass the
dynamic sidebar font size used elsewhere (e.g., replace the literal 10 with the
same variable/property that other sidebar text uses, such as sidebarFontSize or
the shared fontSize binding) so ShortcutHintPill(text: workspaceShortcutLabel,
fontSize: <sidebarFontSize>, emphasis: shortcutHintEmphasis) scales with the
sidebar-font-size setting.
🪄 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: 2ad5f50c-8027-4fb4-9999-7ca7090b811e

📥 Commits

Reviewing files that changed from the base of the PR and between c6d36a5 and bc27aed.

📒 Files selected for processing (4)
  • Sources/ContentView.swift
  • Sources/GhosttyConfig.swift
  • cmuxTests/GhosttyConfigTests.swift
  • cmuxTests/SidebarOrderingTests.swift

Comment thread Sources/GhosttyConfig.swift

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Three fixes from CodeRabbit's review on PR manaflow-ai#2990:

1. NaN/infinity poisoning the clamp. `Double("nan")` and `Double("inf")`
   are valid per IEEE 754 / Apple docs; `min/max` on NaN yields NaN,
   which would have made `sidebarFontScale` NaN → every `N * scale`
   NaN → SwiftUI renders nothing → invisible sidebar. Added
   `size.isFinite` guard and three regression tests: `nan`, `inf`,
   `-infinity`.

2. PR row icons didn't scale. `PullRequestOpenIcon` and
   `PullRequestMergedIcon` are hand-drawn with hardcoded `Path`
   geometry, stroke widths, and node positions. Rather than thread
   `fontScale` through every coordinate (CodeRabbit's proposed diff
   touches ~20 literals, fragile for pixel-grid snapping), apply a
   single `.scaleEffect(fontScale)` with a proportionally-sized frame.
   Behaviorally identical at scale=1, and SwiftUI handles the
   stroke/path/frame transform together. Also scaled the closed-state
   `xmark.circle` frame so all three branches grow together.

3. Workspace shortcut hint pill. The `ShortcutHintPill` call in the
   sidebar trailing accessory hardcoded `fontSize: 10`; now uses
   `10 * fontScale`.

All 18 tests still pass locally. Debug build (`reload.sh --tag
sidebar-font-scale`) compiles clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

13107-13161: ⚠️ Potential issue | 🟡 Minor

Scale the fixed accessory containers with the fonts.

The unread badge, close button, and accessory slot still use fixed 16pt frames while their glyphs now scale. At larger sidebar font sizes, counts/icons can clip or overlap.

Proposed adjustment
         let pullRequestRows: [PullRequestDisplay] = {
             guard detailVisibility.showsPullRequests, let orderedPanelIds else { return [] }
             return pullRequestDisplays(orderedPanelIds: orderedPanelIds)
         }()
+        let scaledAccessorySize = max(16, 16 * fontScale)
+        let scaledTrailingAccessoryWidth = max(trailingAccessoryWidth, scaledAccessorySize)
 
         VStack(alignment: .leading, spacing: 4) {
             HStack(spacing: 8) {
                 if unreadCount > 0 {
                     ZStack {
                         Circle()
                             .fill(activeUnreadBadgeFillColor)
                         Text("\(unreadCount)")
                             .font(.system(size: 9 * fontScale, weight: .semibold))
                             .foregroundColor(.white)
                     }
-                    .frame(width: 16, height: 16)
+                    .frame(width: scaledAccessorySize, height: scaledAccessorySize)
                 }
@@
                     .buttonStyle(.plain)
                     .safeHelp(closeButtonTooltip)
-                    .frame(width: SidebarTrailingAccessoryWidthPolicy.closeButtonWidth, height: 16, alignment: .center)
+                    .frame(width: scaledAccessorySize, height: scaledAccessorySize, alignment: .center)
                     .opacity(showCloseButton && !showsWorkspaceShortcutHint ? 1 : 0)
                     .allowsHitTesting(showCloseButton && !showsWorkspaceShortcutHint)
@@
                 .animation(.easeOut(duration: 0.12), value: showsModifierShortcutHints || alwaysShowShortcutHints)
-                .frame(width: trailingAccessoryWidth, height: 16, alignment: .trailing)
+                .frame(width: scaledTrailingAccessoryWidth, height: scaledAccessorySize, alignment: .trailing)
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 13107 - 13161, The fixed 16pt
accessory frames cause clipping when fonts scale; update the badge, close-button
and trailing accessory container to scale with fontScale (e.g., replace
hard-coded width/height: 16 with a computed size derived from fontScale or the
glyph font sizes). Modify the Circle badge frame, the close button .frame(...)
and the outer .frame(width: trailingAccessoryWidth, height: 16, ...) so their
height/width use a scaled value (e.g., badgeSize = 16 * fontScale or compute
from the font sizes used for the glyphs) and ensure
SidebarTrailingAccessoryWidthPolicy.closeButtonWidth usage is updated or wrapped
to respect the scaled dimensions; keep alignment and hit-testing logic the same.
🤖 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/ContentView.swift`:
- Around line 13107-13161: The fixed 16pt accessory frames cause clipping when
fonts scale; update the badge, close-button and trailing accessory container to
scale with fontScale (e.g., replace hard-coded width/height: 16 with a computed
size derived from fontScale or the glyph font sizes). Modify the Circle badge
frame, the close button .frame(...) and the outer .frame(width:
trailingAccessoryWidth, height: 16, ...) so their height/width use a scaled
value (e.g., badgeSize = 16 * fontScale or compute from the font sizes used for
the glyphs) and ensure SidebarTrailingAccessoryWidthPolicy.closeButtonWidth
usage is updated or wrapped to respect the scaled dimensions; keep alignment and
hit-testing logic the same.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b9e352dc-dfb2-44f0-9ed6-2f6ac8e994e8

📥 Commits

Reviewing files that changed from the base of the PR and between bc27aed and 8f743e0.

📒 Files selected for processing (3)
  • Sources/ContentView.swift
  • Sources/GhosttyConfig.swift
  • cmuxTests/GhosttyConfigTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Sources/GhosttyConfig.swift

Doug Hairfield and others added 2 commits April 19, 2026 10:50
Addresses CodeRabbit finding on manaflow-ai#2990: the unread badge, close button,
and trailing accessory container kept hardcoded 16pt frames while their
inner glyphs were already scaling with fontScale, so at sidebar-font-size
above ~12.5pt the glyphs risked clipping or overlapping.

Introduce `scaledAccessorySize = max(16, 16 * fontScale)` so the floor
at scale <= 1 preserves the existing hit-target size (no-op for default
and smaller fonts), and only grows when the user bumps the sidebar font.
Apply it to the badge circle, close-button frame, and outer trailing
accessory frame. Width of the trailing container uses
`max(trailingAccessoryWidth, scaledAccessorySize)` so the shortcut-hint
pill path keeps its prior layout when fontScale is 1.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Resolves conflicts in Sources/ContentView.swift where origin/main moved
sidebar row precomputations into `workspaceSnapshot` and renamed
`tab.title` → `workspaceSnapshot.title`. Kept the snapshot-sourced
values and re-applied `* fontScale` multipliers added for
sidebar-font-size, plus the `scaledAccessorySize` /
`scaledTrailingAccessoryWidth` helpers.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 12029-12035: The change forces a minimum 16pt trailing gutter even
when there's no trailing accessory; update the calculation so
scaledTrailingAccessoryWidth only applies the 16pt floor when a trailing
accessory exists. In other words, use the existing trailingAccessoryWidth as-is
when it's 0, otherwise compute scaledAccessorySize (max(16, 16 * fontScale)) and
set scaledTrailingAccessoryWidth to the greater of trailingAccessoryWidth and
scaledAccessorySize; reference scaledAccessorySize,
scaledTrailingAccessoryWidth, trailingAccessoryWidth and fontScale when making
the change.
🪄 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: c72717be-f348-45ab-82cf-96d384859176

📥 Commits

Reviewing files that changed from the base of the PR and between 0468628 and d8a340c.

📒 Files selected for processing (3)
  • Sources/ContentView.swift
  • Sources/GhosttyConfig.swift
  • cmuxTests/GhosttyConfigTests.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • cmuxTests/GhosttyConfigTests.swift
  • Sources/GhosttyConfig.swift

Comment thread Sources/ContentView.swift
lawrencecchen and others added 2 commits April 24, 2026 18:35
CodeRabbit review on manaflow-ai#2990: the merge resolution applied max(16, 16 *
fontScale) unconditionally to scaledTrailingAccessoryWidth, which forces
a 16pt gutter on rows where trailingAccessoryWidth was 0 (no close
button AND no shortcut pill). That steals title space from single-
workspace rows that previously had a zero-width trailing gutter.

Keep the 16pt floor only when a trailing accessory is actually shown.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@goodship11

Copy link
Copy Markdown

+1, this would let me actually use cmux at my screen scale!! Thanks for your hard work!

Resolves four conflict regions in Sources/ContentView.swift:

1. SidebarTabItemSettingsSnapshot: keep both new fields
   (sidebarFontScale + iMessageModeEnabled), and adopt main's
   simpler shortcut-hint defaults — main removed the
   sidebarHintXKey/sidebarHintYKey/alwaysShowHintsKey UserDefaults
   plumbing, so HEAD's Self.double(...) calls no longer compile.
2. End-of-init: union both new assignments.
3. Trailing accessory frame: combine main's new
   .shortcutHintVisibilityAnimation modifier with HEAD's
   font-scaled frame width/size.
4. Sidebar PR row: keep main's clickability toggle / localized
   tooltip / accessibility-id structure, while threading
   `fontScale` through PullRequestStatusIcon and the row font.

Drop now-unused `private static func double` helper.

Take main's newer ghostty (22fa801f) and vendor/bonsplit
(f65eccb) submodule pointers — this branch made no intentional
submodule changes.

Verified the Swift compile succeeds locally
(ContentView.o produced); the build only fails at the Zig Run
Script phase due to a Zig 0.15.2 vs 0.16 local environment
mismatch unrelated to this merge.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@livermush

Copy link
Copy Markdown
Author

@coderabbitai resume

Merged origin/main into the branch (resolved conflicts in Sources/ContentView.swift — kept both new sidebarFontScale + iMessageModeEnabled snapshot fields, adopted main's simplified shortcut-hint defaults, threaded fontScale through the new PR-row clickability/accessibility structure, and combined main's .shortcutHintVisibilityAnimation with the font-scaled accessory frame). Submodule pointers updated to main's pin. Ready for another pass.

@coderabbitai

coderabbitai Bot commented May 9, 2026

Copy link
Copy Markdown

@livermush Sure! Resuming reviews and kicking off a fresh pass on the updated branch. The conflict resolution sounds clean — I'll check the merged state of Sources/ContentView.swift carefully, particularly the interaction between sidebarFontScale, iMessageModeEnabled, .shortcutHintVisibilityAnimation, and the font-scaled accessory frame.

✅ Actions performed

Reviews resumed.


Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@livermush

Copy link
Copy Markdown
Author

@austinywang @lawrencecchen — when either of you has a moment, this is ready for a maintainer pass. The branch is now caught up with main (no conflicts) and the implementation has been stable since April; only the bot reviewers have looked at it so far. Issue #2643 is the linked request.

@tylerjbainbridge

Copy link
Copy Markdown

Would love to see this merged 🙏

@tsdevshop

Copy link
Copy Markdown

Adding another +1 to this... my poor eyes are straining hard to read the sidebar text. 🙏

@jasontan

Copy link
Copy Markdown

+1. The font is so small!

@livermush

Copy link
Copy Markdown
Author

Just rebased — conflict with main resolved (5d3311fa1), now MERGEABLE.

Bumping in case this is on anyone's radar: this PR has 6 +1s, the linked issue #2643 has 8 +1s (most recent yesterday), and CodeRabbit approved on May 9. Would appreciate a human review whenever someone has a cycle.

Restore internal visibility so cmuxTests can `@testable import` the
struct. Commit bc27aed ("test(sidebar): expand sidebar-font-size
coverage + make snapshot injectable") originally relaxed this from
`private` to internal, but the access modifier was reverted somewhere
in the merge history. That broke compilation of the existing 6
SidebarTabItemSettingsSnapshotFontScaleTests tests in
cmuxTests/SidebarOrderingTests.swift with `cannot find
'SidebarTabItemSettingsSnapshot' in scope`.

The init's `sidebarFontSizeProvider` parameter is intact; only the
access modifier needed restoring. With this fix all 18 feature tests
pass (12 SidebarFontSizeConfigTests + 6 snapshot scale tests).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@livermush

Copy link
Copy Markdown
Author

Local test results (macOS 26.5 / Xcode 26.5)

Built and ran cmux-unit against the merge head (5d3311fa1 + visibility fix 5a28a35e1) and against the merge's main parent (origin/main@28b0a78d5) for an apples-to-apples baseline.

Feature coverage

All 18 sidebar-font-size tests pass on the PR:

  • SidebarFontSizeConfigTests — 12/12 (parse, clamp, NaN/inf, defaults, injected loader)
  • SidebarTabItemSettingsSnapshotFontScaleTests — 6/6 (default/scaled/clamped/equality)

Suite totals

Pass Fail Skip
origin/main@28b0a78d5 (baseline) 3120 147 29
this PR (5a28a35e1) 3135 150 29

Failure delta

  • 142 of the failures are identical on both sides — pre-existing on main.
  • The ~3-test delta is run-to-run flake noise: the same BrowserDeveloperToolsVisibilityPersistenceTests cases appear on both sides of the diff (i.e. flip between runs).
  • None of the failing tests touch sidebar/font-scale code paths.

About commit 5a28a35e1

Restores internal visibility on SidebarTabItemSettingsSnapshot. The original snapshot-injectable change in bc27aedd6 made it internal so the test target could @testable import it, but that was reverted to private somewhere in the merge history, breaking compilation of the existing 6 snapshot scale tests. One-character fix; init signature with sidebarFontSizeProvider was already correct.

🤖 Generated with Claude Code

@planetf1

Copy link
Copy Markdown

Thanks for doing this -- ++ on the need for this (eyesight). great addition

Resolved conflict in Sources/ContentView.swift: main consolidated the
workspace-row shortcut-hint pill into the shared sidebarShortcutHintOverlay
modifier (replacing the old inline .topTrailing pill with EmptyView). Adopted
main's structure and threaded fontScale through sidebarShortcutHintOverlay
(default 1.0) so the hint pill still scales with sidebar-font-size while
keeping a single pill render path.

Submodule pointers (ghostty, vendor/bonsplit) match main.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@livermush

Copy link
Copy Markdown
Author

Rebased onto latest main (0241ddd51) — conflict in ContentView.swift resolved, now MERGEABLE, builds clean locally (Xcode 26.5 / macOS 26.5), 0 unresolved review threads, CodeRabbit approved.

The only thing blocking merge is CI gating: the four required checks (web-typecheck, workflow-guard-tests, web-db-migrations, remote-daemon-tests) are stuck at action_required behind the fork-PR "Approve and run workflows" button — they can't start until a maintainer clicks it. This is a one-click unblock, not a code-review request.

Could a maintainer hit "Approve and run workflows" in the Checks section? Required approving reviews on main is 0, so once those four run green this should be mergeable. 🙏

cc @lawrencecchen @austinywang @azooz2003-bit

@lugfug

lugfug commented May 30, 2026

Copy link
Copy Markdown

+1, this is exactly the gap I hit. I run many parallel Claude Code / Codex sessions and the sidebar tab text is too small to scan across tabs, with no way to size it today — Cmd +/- and Ghostty's font-size only affect terminal content, not the sidebar chrome. A sidebar-font-size option would solve it directly. Is anything blocking this from landing? Happy to test a build.

@livermush

Copy link
Copy Markdown
Author

Closing this — the feature shipped via #4798, which landed sidebar (and surface-tab-bar) font sizing with a Settings slider, CLI support, and docs, and closed #2643 as completed. Thanks to @austinywang for getting it across the line, and to everyone who +1'd here. 🎉

For anyone arriving from the issue: set it in ~/.config/cmux/cmux.json via sidebar-font-size (points, 10–20, default 12.5) or the new Settings → Sidebar slider.

@livermush livermush closed this Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow customizing sidebar workspace tab label font size

8 participants