Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 50 additions & 7 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4637,10 +4637,12 @@ struct CMUXCLI {
let (surfaceOpt, argsAfterSurface) = parseOption(argsAfterWindow, name: "--surface")
let (directionOpt, argsAfterDirection) = parseOption(argsAfterSurface, name: "--direction")
let (focusOpt, argsAfterFocus) = parseOption(argsAfterDirection, name: "--focus")
args = argsAfterFocus
let (fontSizeOpt, argsAfterFontSize) = parseOption(argsAfterFocus, name: "--font-size")
args = argsAfterFontSize

// Determine subcommand. Explicit "open" is supported, otherwise treat
// a single positional argument as shorthand path.
let usage = "cmux markdown open <path> [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--direction right|down|left|up] [--focus <true|false>] [--font-size <points>]"
let subArgs: [String]
if let first = args.first, first.lowercased() == "open" {
subArgs = Array(args.dropFirst())
Expand All @@ -4652,7 +4654,7 @@ struct CMUXCLI {
if let first = args.first, first.hasPrefix("-") {
throw CLIError(
message:
"markdown open: unknown flag '\(first)'. Usage: cmux markdown open <path> [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--direction right|down|left|up] [--focus <true|false>]"
"markdown open: unknown flag '\(first)'. Usage: \(usage)"
)
} else if let first = args.first, looksLikePath(first) || first.contains(".") {
subArgs = args
Expand All @@ -4664,22 +4666,29 @@ struct CMUXCLI {
}

guard let rawPath = subArgs.first, !rawPath.isEmpty else {
throw CLIError(message: "markdown open requires a file path. Usage: cmux markdown open <path>")
throw CLIError(message: "markdown open requires a file path. Usage: \(usage)")
}
let trailingArgs = Array(subArgs.dropFirst())
if let unknownFlag = trailingArgs.first(where: { $0.hasPrefix("-") }) {
throw CLIError(
message:
"markdown open: unknown flag '\(unknownFlag)'. Usage: cmux markdown open <path> [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--direction right|down|left|up] [--focus <true|false>]"
"markdown open: unknown flag '\(unknownFlag)'. Usage: \(usage)"
)
}
if let extraArg = trailingArgs.first {
throw CLIError(
message:
"markdown open: unexpected argument '\(extraArg)'. Usage: cmux markdown open <path> [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--direction right|down|left|up] [--focus <true|false>]"
"markdown open: unexpected argument '\(extraArg)'. Usage: \(usage)"
)
}

let fontSizePoints: Double?
if let fontSizeOpt {
fontSizePoints = try parseMarkdownViewerFontSize(fontSizeOpt)
} else {
fontSizePoints = nil
}

let absolutePath = resolvePath(rawPath)

// Build params
Expand All @@ -4702,6 +4711,9 @@ struct CMUXCLI {
}
}
try applyFocusOption(focusOpt, defaultValue: false, to: &params)
if let fontSizePoints {
params["font_size"] = fontSizePoints
}

let payload = try client.sendV2(method: "markdown.open", params: params)

Expand All @@ -4711,7 +4723,9 @@ struct CMUXCLI {
let surfaceText = formatHandle(payload, kind: "surface", idFormat: idFormat) ?? "unknown"
let paneText = formatHandle(payload, kind: "pane", idFormat: idFormat) ?? "unknown"
let filePath = (payload["path"] as? String) ?? absolutePath
print("OK surface=\(surfaceText) pane=\(paneText) path=\(filePath)")
let fontSize = doubleFromAny(payload["font_size"]) ?? fontSizePoints
let fontSizeText = fontSize.map { " font_size=\(formatMarkdownViewerFontSize($0))" } ?? ""
print("OK surface=\(surfaceText) pane=\(paneText) path=\(filePath)\(fontSizeText)")
}
}

Expand Down Expand Up @@ -14436,12 +14450,14 @@ struct CMUXCLI {
--window <id|ref|index> Target window
--direction <left|right|up|down> Split direction (default: right)
--focus <true|false> Focus the markdown panel (default: false)
--font-size <points> Rendered markdown font size

Examples:
cmux markdown open plan.md
cmux markdown ~/project/CHANGELOG.md
cmux markdown open ./docs/design.md --workspace 0
cmux markdown open plan.md --direction down
cmux markdown open plan.md --font-size 12
"""
default:
return nil
Expand Down Expand Up @@ -14500,6 +14516,33 @@ struct CMUXCLI {
return (value, remaining)
}

private func parseMarkdownViewerFontSize(_ rawValue: String) throws -> Double {
let trimmed = rawValue.trimmingCharacters(in: .whitespacesAndNewlines)
guard let size = Double(trimmed),
isUsableMarkdownViewerFontSize(size) else {
throw CLIError(message: "--font-size must be a positive number no larger than 96")
}
return roundedMarkdownViewerMetric(size)
}

private func isUsableMarkdownViewerFontSize(_ size: Double) -> Bool {
size.isFinite && size > 0 && size <= 96
Comment on lines +14519 to +14529

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align --font-size validation with the new 1–96pt contract.

This helper currently accepts any finite value greater than 0, so cmux markdown open --font-size 0.5 passes here even though the new markdown viewer font-size setting is documented as 1–96 pt. That makes the CLI surface inconsistent with the settings/schema layer for the same feature.

Suggested fix
     private func parseMarkdownViewerFontSize(_ rawValue: String) throws -> Double {
         let trimmed = rawValue.trimmingCharacters(in: .whitespacesAndNewlines)
         guard let size = Double(trimmed),
               isUsableMarkdownViewerFontSize(size) else {
-            throw CLIError(message: "--font-size must be a positive number no larger than 96")
+            throw CLIError(message: "--font-size must be between 1 and 96")
         }
         return roundedMarkdownViewerMetric(size)
     }

     private func isUsableMarkdownViewerFontSize(_ size: Double) -> Bool {
-        size.isFinite && size > 0 && size <= 96
+        size.isFinite && size >= 1 && size <= 96
     }
🤖 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 `@CLI/cmux.swift` around lines 14519 - 14529, The CLI currently allows font
sizes >0 (so 0.5 passes) but the documented/required contract is 1–96 pt; update
the validation in isUsableMarkdownViewerFontSize to require size.isFinite &&
size >= 1 && size <= 96 (instead of size > 0) and adjust the
parseMarkdownViewerFontSize error message to reflect "must be between 1 and 96"
(or "1–96") so the CLI message matches the settings/schema; keep using
roundedMarkdownViewerMetric(size) on success.

}

private func roundedMarkdownViewerMetric(_ value: Double) -> Double {
(value * 100).rounded() / 100
}

private func formatMarkdownViewerFontSize(_ value: Double) -> String {
let rounded = roundedMarkdownViewerMetric(value)
if rounded.rounded() == rounded {
return String(Int(rounded))
}
return String(format: "%.2f", rounded)
.replacingOccurrences(of: #"0+$"#, with: "", options: .regularExpression)
.replacingOccurrences(of: #"\.$"#, with: "", options: .regularExpression)
}

private func parseRepeatedOption(_ args: [String], name: String) -> ([String], [String]) {
var remaining: [String] = []
var values: [String] = []
Expand Down Expand Up @@ -31402,7 +31445,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
respawn-pane [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--command <cmd>]
display-message [-p|--print] <text>

markdown [open] <path> [--focus <true|false>] (open markdown file in formatted viewer panel with live reload)
markdown [open] <path> [--focus <true|false>] [--font-size <points>] (open markdown file in formatted viewer panel with live reload)
diff [patch-file|-] [--source <unstaged|staged|branch|last-turn>] [--cwd <path>] [--base <ref>] [--focus <true|false>] [--no-focus] [--title <text>] [--layout <split|unified>] [--font-size <points>] (open patch input or git source in a browser split)

browser [--surface <id|ref|index> | <surface>] <subcommand> ...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ public struct AppCatalogSection: SettingCatalogSection {
userDefaultsKey: "openMarkdownInCmuxViewer"
)

public let markdownViewerFontSize = DefaultsKey<Double>(
id: "app.markdownViewerFontSize",
defaultValue: 15,
userDefaultsKey: "markdownViewerFontSize"
)
Comment on lines +82 to +86

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Document the new public settings key.

markdownViewerFontSize is a new public symbol in a package, but it has no Swift-DocC comment.

📝 Proposed fix
+    /// Default font size, in points, for markdown viewer panels.
     public let markdownViewerFontSize = DefaultsKey<Double>(
         id: "app.markdownViewerFontSize",
         defaultValue: 15,
         userDefaultsKey: "markdownViewerFontSize"
     )

As per coding guidelines, “Every public symbol in new Swift packages under Packages/ must be documented with Swift-DocC triple-slash comment at time of writing.”

📝 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
public let markdownViewerFontSize = DefaultsKey<Double>(
id: "app.markdownViewerFontSize",
defaultValue: 15,
userDefaultsKey: "markdownViewerFontSize"
)
/// Default font size, in points, for markdown viewer panels.
public let markdownViewerFontSize = DefaultsKey<Double>(
id: "app.markdownViewerFontSize",
defaultValue: 15,
userDefaultsKey: "markdownViewerFontSize"
)
🤖 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 `@Packages/CmuxSettings/Sources/CmuxSettings/Keys/AppCatalogSection.swift`
around lines 82 - 86, Add a Swift-DocC triple-slash comment for the new public
DefaultsKey symbol `markdownViewerFontSize` in `AppCatalogSection.swift`; the
comment should briefly describe the setting's purpose (controls the markdown
viewer font size), note the default value (15) and the userDefaults key
(`markdownViewerFontSize`), and include usage guidance (units/type: Double) so
the public symbol `markdownViewerFontSize: DefaultsKey<Double>` is properly
documented per package guidelines.


public let iMessageMode = DefaultsKey<Bool>(
id: "app.iMessageMode",
defaultValue: false,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Foundation

public enum CmuxSettingsRuntimeNotifications {
public static let markdownViewerFontSizeDidChange = Notification.Name("cmux.markdownViewerFontSizeDidChange")
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ extension ShortcutAction {
case .browserZoomIn: return ShortcutStroke(key: "=", command: true)
case .browserZoomOut: return ShortcutStroke(key: "-", command: true)
case .browserZoomReset: return ShortcutStroke(key: "0", command: true)
case .markdownZoomIn: return ShortcutStroke(key: "=", command: true)
case .markdownZoomOut: return ShortcutStroke(key: "-", command: true)
case .markdownZoomReset: return ShortcutStroke(key: "0", command: true)
case .find: return ShortcutStroke(key: "f", command: true)
case .findInDirectory: return ShortcutStroke(key: "f", command: true, shift: true)
case .findNext: return ShortcutStroke(key: "g", command: true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ public enum ShortcutAction: String, CaseIterable, Sendable, Hashable, SettingCod
case browserZoomIn
case browserZoomOut
case browserZoomReset
case markdownZoomIn
case markdownZoomOut
case markdownZoomReset
case find
case findInDirectory
case findNext
Expand All @@ -106,6 +109,7 @@ extension ShortcutAction {
case navigation
case panes
case browser
case markdown

public var title: String {
switch self {
Expand All @@ -114,6 +118,7 @@ extension ShortcutAction {
case .navigation: return "Navigation"
case .panes: return "Panes"
case .browser: return "Browser & Find"
case .markdown: return "Markdown Viewer"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Localize the new shortcut metadata.

These new Group.title / displayName values are hard-coded English, and KeyboardShortcutsSection renders action.displayName directly, so the Markdown shortcut rows stay untranslated even when the rest of Settings is localized.

As per coding guidelines, "For production user-facing text, fail partial localization: Swift text must use localized APIs with matching translated string-catalog entries."

Also applies to: 231-233

🤖 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 `@Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift` at
line 121, The hard-coded English titles for shortcut metadata (e.g., the
.markdown case returning "Markdown Viewer" in ShortcutAction.swift and the
similar strings at the other noted cases) must be replaced with localized
strings and corresponding catalog entries; update the Group.title / displayName
providers in ShortcutAction (and the other occurrences referenced at 231-233) to
use a localization API (e.g., NSLocalizedString or String(localized:)) with
unique keys like "ShortcutAction.Markdown.title" /
"ShortcutAction.Markdown.displayName", then add matching entries to the app's
Localizable.strings (or the SwiftGen/localizable catalog) so
KeyboardShortcutsSection rendering action.displayName shows translated text.
Ensure keys are descriptive and consistent across all cases.

}
}
}
Expand Down Expand Up @@ -148,6 +153,8 @@ extension ShortcutAction {
.hideFind, .useSelectionForFind, .toggleBrowserDeveloperTools,
.showBrowserJavaScriptConsole, .toggleReactGrab:
return .browser
case .markdownZoomIn, .markdownZoomOut, .markdownZoomReset:
return .markdown
}
}

Expand Down Expand Up @@ -221,6 +228,9 @@ extension ShortcutAction {
case .browserZoomIn: return "Zoom In"
case .browserZoomOut: return "Zoom Out"
case .browserZoomReset: return "Actual Size"
case .markdownZoomIn: return "Markdown Viewer: Zoom In"
case .markdownZoomOut: return "Markdown Viewer: Zoom Out"
case .markdownZoomReset: return "Markdown Viewer: Actual Size"
case .find: return "Find…"
case .findInDirectory: return "Find in Directory…"
case .findNext: return "Find Next"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ extension Array where Element == CuratedSettingEntry {
.init(section: .app, id: "preferred-editor", title: "Open Files With", synonyms: "app.preferredEditor editor open file code vscode visual studio zed sublime subl cursor"),
.init(section: .app, id: "supported-file-previews", title: "Open Supported Files in cmux", synonyms: "app.openSupportedFilesInCmux cmd click file preview pdf image video audio quicklook quick look editor external"),
.init(section: .app, id: "markdown-viewer", title: "Open Markdown in cmux Viewer", synonyms: "app.openMarkdownInCmuxViewer md markdown mdx viewer preview readme"),
.init(section: .app, id: "markdown-font-size", title: "Markdown Viewer Font Size", synonyms: "app.markdownViewerFontSize markdown viewer font size text scale zoom points preview"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't add English-only curated setting entries.

This new curated entry hard-codes both the visible search-result title and its search terms in English, so localized users will see an untranslated result here and won't get locale-appropriate search matches for the new setting.

As per coding guidelines, "For production user-facing text, fail partial localization: Swift text must use localized APIs with matching translated string-catalog entries."

🤖 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
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry`+Default.swift
at line 44, The curated entry created via CuratedSettingEntry.init(section:
.app, id: "markdown-font-size", title: "Markdown Viewer Font Size", synonyms:
"app.markdownViewerFontSize markdown viewer font size text scale zoom points
preview") must not hard-code English; replace the literal title and synonyms
with localized lookups (e.g., NSLocalizedString or localized string key API) and
add corresponding keys/values to the app's Localizable.strings catalog for all
supported locales. Update the CuratedSettingEntry creation to reference the
localized title key and a localized synonyms string (or localized array) and
ensure matching entries exist in the strings catalog so both the visible title
and search terms are localized consistently.

.init(section: .app, id: "terminal-config", title: "Terminal Config", synonyms: "ghostty config merged generated preview terminal configuration window open config"),
.init(section: .app, id: "imessage-mode", title: "iMessage Mode", synonyms: "app.iMessageMode imessage message messages chat prompt prompts submitted texting reorder move workspace top agent send"),
.init(section: .app, id: "reorder-notification", title: "Reorder on Notification", synonyms: "app.reorderOnNotification notification reorder move workspace top unread sort"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import UniformTypeIdentifiers
@MainActor
public struct AppSection: View {
private let catalog: SettingCatalog
private let defaultsStore: UserDefaultsSettingsStore
private let hostActions: SettingsHostActions

// Every bound value-model lives here as view state, constructed once
Expand All @@ -35,6 +36,7 @@ public struct AppSection: View {
@State private var preferredEditor: DefaultsValueModel<String>
@State private var openSupported: DefaultsValueModel<Bool>
@State private var openMarkdown: DefaultsValueModel<Bool>
@State private var markdownFontSize: DefaultsValueModel<Double>
@State private var iMessage: DefaultsValueModel<Bool>
@State private var reorder: DefaultsValueModel<Bool>
@State private var dockBadge: DefaultsValueModel<Bool>
Expand Down Expand Up @@ -62,6 +64,7 @@ public struct AppSection: View {
hostActions: SettingsHostActions
) {
self.catalog = catalog
self.defaultsStore = defaultsStore
self.hostActions = hostActions
_language = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.language))
_appearance = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.appearance))
Expand All @@ -75,6 +78,7 @@ public struct AppSection: View {
_preferredEditor = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.preferredEditor))
_openSupported = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.openSupportedFilesInCmux))
_openMarkdown = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.openMarkdownInCmuxViewer))
_markdownFontSize = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.markdownViewerFontSize))
_iMessage = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.iMessageMode))
_reorder = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.app.reorderOnNotification))
_dockBadge = State(initialValue: DefaultsValueModel(store: defaultsStore, key: catalog.notifications.dockBadge))
Expand All @@ -96,6 +100,7 @@ public struct AppSection: View {

private static let columnWidth: CGFloat = 196
private static let notificationSoundControlWidth: CGFloat = 280
private static let markdownFontSizeRange: ClosedRange<Double> = 1...96

/// Languages legacy `AppLanguage` exposes (cmuxApp.swift line
/// 4338). The shared `CmuxSettings.AppLanguage` adds `.vi` for a
Expand Down Expand Up @@ -311,6 +316,35 @@ public struct AppSection: View {
}
SettingsCardDivider()

// Markdown Viewer Font Size
SettingsCardRow(
configurationReview: .json("app.markdownViewerFontSize"),
String(localized: "settings.app.markdownViewerFontSize", defaultValue: "Markdown Viewer Font Size"),
subtitle: String(localized: "settings.app.markdownViewerFontSize.subtitle", defaultValue: "Default rendered markdown size for new viewer panels. Panel zoom shortcuts override this per panel until reset."),
controlWidth: 140
) {
Stepper(
value: Binding(
get: { Self.clampedMarkdownFontSize(markdownFontSize.current) },
set: { setMarkdownFontSize($0) }
),
in: Self.markdownFontSizeRange,
step: 1
) {
Text(
String.localizedStringWithFormat(
String(localized: "settings.fontSize.valuePoints", defaultValue: "%@ pt"),
Self.formatMarkdownFontSize(markdownFontSize.current)
)
)
.foregroundStyle(.secondary)
.monospacedDigit()
.frame(minWidth: 44, alignment: .trailing)
}
.accessibilityIdentifier("SettingsMarkdownViewerFontSizeStepper")
}
SettingsCardDivider()

// iMessage Mode
SettingsCardRow(
configurationReview: .json("app.iMessageMode"),
Expand Down Expand Up @@ -369,6 +403,7 @@ public struct AppSection: View {
Toggle("", isOn: Binding(get: { showInMenuBar.current }, set: { showInMenuBar.set($0) }))
.labelsHidden()
.controlSize(.small)
.accessibilityIdentifier("SettingsShowInMenuBarToggle")
}
.disabled(menuBarOnly.current)
SettingsCardDivider()
Expand Down Expand Up @@ -715,6 +750,33 @@ public struct AppSection: View {
}
}

private static func clampedMarkdownFontSize(_ value: Double) -> Double {
guard value.isFinite else { return 15 }
return min(max(value, markdownFontSizeRange.lowerBound), markdownFontSizeRange.upperBound)
}

private static func formatMarkdownFontSize(_ value: Double) -> String {
let clamped = clampedMarkdownFontSize(value)
if clamped.rounded() == clamped {
return "\(Int(clamped))"
}
return String(format: "%.2f", clamped)
}
Comment on lines +758 to +764

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Swift/Foundation, what locale behavior does String(format: "%.2f", value) use for user-facing decimal output, and what locale-aware API is recommended for formatting floating-point values in SwiftUI settings labels?

💡 Result:

The Swift initializer String(format: "%.2f", value) does not automatically respect the user's current locale settings [1]. It uses a C-style printf-based formatting engine that defaults to the "C" locale, meaning it will consistently use a period (.) as the decimal separator regardless of the user's regional preferences, which may expect a comma (,) or other locale-specific conventions [1]. To make String(format:) locale-aware, you must explicitly provide a locale, such as by using the initializer String(format:locale:_:arguments:) [1][2]. For SwiftUI settings labels and general UI output, the recommended approach is to use the modern Swift FormatStyle APIs [3][4]. These APIs are natively locale-aware and handle formatting details automatically [5][6]. Recommended API: The.formatted method with a specific format style is the standard for SwiftUI [5][3]. For example, to format a number to two decimal places while respecting the current user's locale, you can use: Text(value, format:.number.precision(.fractionLength(2))) This approach is preferred because it is declarative, type-safe, and automatically adapts to the user's environment (e.g., decimal separators, grouping separators) without requiring manual locale management [5][6][4].

Citations:


Use locale-aware number formatting for displayed font size

String(format: "%.2f", clamped) uses the C/printf-style formatting behavior (not the user’s current locale), so the decimal separator can be wrong for some locales. Format the Double with Swift’s locale-aware FormatStyle (e.g., clamped.formatted(.number.precision(.fractionLength(2)))) and apply the same locale-aware formatting to the integer path too in Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swift (lines 758-764).

🤖 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 `@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swift`
around lines 758 - 764, The function formatMarkdownFontSize currently uses
String(format:) which is not locale-aware; replace both the integer and
fractional return paths in formatMarkdownFontSize(_:) to use Swift's
locale-aware FormatStyle (e.g., use
clamped.formatted(.number.precision(.fractionLength(0))) for integer-equivalent
values and clamped.formatted(.number.precision(.fractionLength(2))) for the
fractional case), keeping clamped computed via clampedMarkdownFontSize(_:) and
returning the formatted string.


private func setMarkdownFontSize(_ value: Double) {
let clamped = Self.clampedMarkdownFontSize(value)
let key = catalog.app.markdownViewerFontSize
Task { [defaultsStore] in
await defaultsStore.set(clamped, for: key)
await MainActor.run {
NotificationCenter.default.post(
name: CmuxSettingsRuntimeNotifications.markdownViewerFontSizeDidChange,
object: nil
)
}
}
Comment on lines +766 to +777

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Update the bound value model synchronously.

This setter bypasses markdownFontSize and only writes through defaultsStore in a fire-and-forget task. That breaks the file’s own “value-model drives invalidation” contract, so the stepper label/value can lag or snap back until store observation catches up. Route the change through the bound DefaultsValueModel as part of the setter, then post the runtime notification from the persisted path.

🤖 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 `@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swift`
around lines 766 - 777, The setter setMarkdownFontSize(_:) currently only writes
to defaultsStore inside a Task and never updates the in-memory bound model,
causing UI lag; change it to synchronously assign the clamped value to the bound
DefaultsValueModel (markdownFontSize) first, then persist that same clamped
value to defaultsStore (using the existing key and Task/await pattern), and
finally post CmuxSettingsRuntimeNotifications.markdownViewerFontSizeDidChange
from the persisted path (after await) on MainActor so observers see the
persisted change; update references to setMarkdownFontSize, markdownFontSize,
defaultsStore, key, and the NotificationCenter.post call accordingly.

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stepper setter bypasses model causing async update lag

Medium Severity

setMarkdownFontSize writes to the store via an async Task instead of calling markdownFontSize.set() directly like every other settings binding in this file. The Stepper's getter reads markdownFontSize.current, which won't reflect the new value until the async store write propagates back through observation. This can cause the Stepper to visually stutter or snap back momentarily on each click, since SwiftUI expects binding updates to be synchronous.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 622ad8b. Configure here.


private func confirmQuitSubtitle(_ mode: ConfirmQuitMode) -> String {
// Mirrors legacy confirmQuitModeSubtitle keys/text.
switch mode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,44 @@ public struct KeyboardShortcutsSection: View {

// MARK: - Conflict helpers

private enum ShortcutConflictContext: Equatable {
case application
case nonBrowserPanel
case browserPanel
case markdownPanel
case rightSidebarFocus

func overlaps(_ other: ShortcutConflictContext) -> Bool {
if self == .application || other == .application {
return true
}
if self == .nonBrowserPanel && other == .markdownPanel {
return true
}
if self == .markdownPanel && other == .nonBrowserPanel {
return true
}
return self == other
}
}

private static func shortcutContext(for action: ShortcutAction) -> ShortcutConflictContext {
switch action {
case .switchRightSidebarToFiles, .switchRightSidebarToFind, .switchRightSidebarToSessions,
.switchRightSidebarToFeed, .switchRightSidebarToDock:
return .rightSidebarFocus
case .renameTab, .renameWorkspace:
return .nonBrowserPanel
case .browserBack, .browserForward, .browserReload, .toggleBrowserDeveloperTools,
.showBrowserJavaScriptConsole, .browserZoomIn, .browserZoomOut, .browserZoomReset:
return .browserPanel
case .markdownZoomIn, .markdownZoomOut, .markdownZoomReset:
return .markdownPanel
default:
return .application
}
}
Comment on lines +293 to +329

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Run restores through the same conflict gate.

The new context-aware detectConflict path only protects fresh recordings. A restored shortcut still goes straight through restoreBinding, so users can clear action A, bind its old chord to overlapping action B, then hit Restore on A and persist a conflict the recorder is supposed to reject.

🔧 Suggested fix
     private func restoreBinding(_ shortcut: StoredShortcut, for action: ShortcutAction) async {
+        if let conflict = detectConflict(for: action, stroke: shortcut) {
+            conflictRejections[action.rawValue] = conflict
+            bareKeyRejections.remove(action.rawValue)
+            return
+        }
         var updated = bindings
         updated[action.rawValue] = shortcut
         restoreShortcuts.removeValue(forKey: action.rawValue)
         conflictRejections.removeValue(forKey: action.rawValue)
         await write(updated)

Also applies to: 346-348

🤖 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
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swift`
around lines 293 - 329, Restoring a shortcut currently bypasses the new
context-aware conflict check and can persist conflicts; update the restore flow
so restoreBinding (and any restore path around lines 346-348) invokes the same
conflict detection used for fresh recordings: call the existing detectConflict
routine (or replicate its logic) using shortcutContext(for:) and
ShortcutConflictContext.overlaps(_:) before applying/persisting the restored
binding, and abort the restore (show error/skip persist) when a conflict is
detected so restored shortcuts pass through the same gate as recorded ones.


/// Mirrors legacy `KeyboardShortcutSettings.Action.conflicts(with:proposedAction:configuredShortcut:)`
/// at a coarser grain: only treat two actions as conflicting when the
/// *configured* (effective) shortcut of the other action is not
Expand All @@ -305,7 +343,9 @@ public struct KeyboardShortcutsSection: View {
}

private func detectConflict(for action: ShortcutAction, stroke: StoredShortcut) -> ShortcutAction? {
let context = Self.shortcutContext(for: action)
for other in ShortcutAction.allCases where other != action {
guard context.overlaps(Self.shortcutContext(for: other)) else { continue }
let override = bindings[other.rawValue]
let effective = override ?? other.defaultStroke.map { StoredShortcut(first: $0) }
guard let effective, !effective.isUnbound else { continue }
Expand Down
Loading
Loading