From 0300f9115bec6e728006bd30f1345a97db1a1457 Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:02:21 -0300 Subject: [PATCH 01/11] refactor(settings): drop the sidebar tint controls from Settings Four of the six rows in Sidebar Appearance were per-pixel tuning of one surface: light tint hex, dark tint hex, tint opacity, and a section-local reset. The same four keys are already bound by the Debug window, so this removes the copy that shipped to every user and keeps the one intended for development. The @AppStorage bindings stay on SettingsView because "Reset all settings" still writes them, which is what restores anyone who set a tint while the rows existed. Sidebar Appearance goes from six rows to two: Match Terminal Background and Show Claude Quota, both of which change behaviour rather than pixels. Also drops the nine localization keys that went dead with the rows. settings.sidebarAppearance.defaultLabel is deliberately kept -- the Workspace Colors rows still use it. --- Resources/Localizable.xcstrings | 101 +------------------------------- Sources/SettingsView.swift | 81 ++----------------------- 2 files changed, 7 insertions(+), 175 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index daccbfa1..51995e87 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -10643,39 +10643,6 @@ } } }, - "settings.sidebarAppearance.reset": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Reset Sidebar Tint" - } - } - } - }, - "settings.sidebarAppearance.reset.button": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Reset" - } - } - } - }, - "settings.sidebarAppearance.reset.subtitle": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Restore default sidebar appearance." - } - } - } - }, "settings.sidebarAppearance.showClaudeQuota": { "extractionState": "manual", "localizations": { @@ -10710,72 +10677,6 @@ } } }, - "settings.sidebarAppearance.tintColorDark": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Dark Mode Tint" - } - } - } - }, - "settings.sidebarAppearance.tintColorDark.subtitle": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Sidebar tint color when using dark appearance." - } - } - } - }, - "settings.sidebarAppearance.tintColorLight": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Light Mode Tint" - } - } - } - }, - "settings.sidebarAppearance.tintColorLight.subtitle": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Sidebar tint color when using light appearance." - } - } - } - }, - "settings.sidebarAppearance.tintOpacity": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Tint Opacity" - } - } - } - }, - "settings.sidebarAppearance.tintOpacity.subtitle": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "How strongly the tint color shows over the sidebar material." - } - } - } - }, "settings.state.active": { "extractionState": "manual", "localizations": { @@ -13182,4 +13083,4 @@ } }, "version": "1.0" -} \ No newline at end of file +} diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index 154876ed..3990f549 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -1149,82 +1149,13 @@ struct SettingsView: View { String(localized: "settings.sidebarAppearance.showClaudeQuota", defaultValue: "Show Claude Quota") ) } - - SettingsCardDivider() - - SettingsCardRow( - String(localized: "settings.sidebarAppearance.tintColorLight", defaultValue: "Light Mode Tint"), - subtitle: String(localized: "settings.sidebarAppearance.tintColorLight.subtitle", defaultValue: "Sidebar tint color when using light appearance.") - ) { - HStack(spacing: 8) { - HexColorPicker( - hex: sidebarTintHexLight ?? sidebarTintHex, - fallback: .black - ) { newHex in - sidebarTintHexLight = newHex - } - - Text(sidebarTintHexLight ?? String(localized: "settings.sidebarAppearance.defaultLabel", defaultValue: "Default")) - .font(.system(size: 12, weight: .medium, design: .monospaced)) - .foregroundStyle(.secondary) - .frame(width: 76, alignment: .trailing) - } - } - - SettingsCardDivider() - - SettingsCardRow( - String(localized: "settings.sidebarAppearance.tintColorDark", defaultValue: "Dark Mode Tint"), - subtitle: String(localized: "settings.sidebarAppearance.tintColorDark.subtitle", defaultValue: "Sidebar tint color when using dark appearance.") - ) { - HStack(spacing: 8) { - HexColorPicker( - hex: sidebarTintHexDark ?? sidebarTintHex, - fallback: .black - ) { newHex in - sidebarTintHexDark = newHex - } - - Text(sidebarTintHexDark ?? String(localized: "settings.sidebarAppearance.defaultLabel", defaultValue: "Default")) - .font(.system(size: 12, weight: .medium, design: .monospaced)) - .foregroundStyle(.secondary) - .frame(width: 76, alignment: .trailing) - } - } - - SettingsCardDivider() - - SettingsCardRow( - String(localized: "settings.sidebarAppearance.tintOpacity", defaultValue: "Tint Opacity"), - subtitle: String(localized: "settings.sidebarAppearance.tintOpacity.subtitle", defaultValue: "How strongly the tint color shows over the sidebar material.") - ) { - HStack(spacing: 8) { - Slider(value: $sidebarTintOpacity, in: 0...1) - .frame(width: 140) - Text(String(format: "%.0f%%", sidebarTintOpacity * 100)) - .font(.system(size: 12, weight: .medium, design: .monospaced)) - .foregroundStyle(.secondary) - .frame(width: 36, alignment: .trailing) - } - } - - SettingsCardDivider() - - SettingsCardRow( - String(localized: "settings.sidebarAppearance.reset", defaultValue: "Reset Sidebar Tint"), - subtitle: String(localized: "settings.sidebarAppearance.reset.subtitle", defaultValue: "Restore default sidebar appearance.") - ) { - Button(String(localized: "settings.sidebarAppearance.reset.button", defaultValue: "Reset")) { - sidebarTintHexLight = nil - sidebarTintHexDark = nil - sidebarTintHex = SidebarTintDefaults.hex - sidebarTintOpacity = SidebarTintDefaults.opacity - } - .buttonStyle(.bordered) - .controlSize(.small) - } + // Light/dark tint hex, tint opacity and a section-local reset used to + // live here. They were per-pixel tuning of one surface, shipped to + // every user, and the same four keys are already bound by the Debug + // window (DebugWindows.swift) where that kind of tuning belongs. The + // bindings stay on this view because "Reset all settings" below still + // restores them for anyone who set a value while the rows existed. } - } @ViewBuilder From f9b0b899cf94fc8f1faf11edcc6ffd66ee62298f Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:13:06 -0300 Subject: [PATCH 02/11] refactor(settings): drop the workspace color pickers and palette editor Workspace Colors carried a full palette CRUD editor inside the settings panel: a hex picker and Remove button per named color, plus a note telling you to edit settings.json for the same thing. It also had selection-highlight and notification-badge hex pickers, which are the same per-pixel tuning the sidebar tints were. All of it goes. settings.json stays the way to manage named colors, which is what the note already said. "Reset all settings" still calls WorkspaceTabColorSettings.reset() and nils both hex keys, so anything set while these rows existed is recoverable. The section keeps one row, Workspace Color Indicator, which changes what the sidebar draws rather than what shade it draws it in. Removing the editor made a whole chain dead: workspaceTabPaletteEntries and the four helpers that maintained it, an onAppear refresh, and an onReceive on UserDefaults.didChangeNotification that reloaded the palette on every defaults write. That last one ran for every settings change while the window was open. Also drops nine localization keys, including settings.sidebarAppearance.defaultLabel, whose last two users were the hex pickers removed here. --- Resources/Localizable.xcstrings | 99 -------------------- Sources/SettingsView.swift | 161 ++------------------------------ 2 files changed, 7 insertions(+), 253 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 51995e87..4b84a809 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -10610,17 +10610,6 @@ } } }, - "settings.sidebarAppearance.defaultLabel": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Default" - } - } - } - }, "settings.sidebarAppearance.matchTerminalBackground": { "extractionState": "manual", "localizations": { @@ -10721,50 +10710,6 @@ } } }, - "settings.workspaceColors.base": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Base: %@" - } - } - } - }, - "settings.workspaceColors.customEntry": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Named palette entry." - } - } - } - }, - "settings.workspaceColors.dictionaryNote": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Edit settings.json to add or remove named colors. \"Choose Custom Color...\" still adds local Custom N entries." - } - } - } - }, - "settings.workspaceColors.emptyPalette": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "No palette entries. Add colors in settings.json or use \"Choose Custom Color...\" from a workspace context menu." - } - } - } - }, "settings.workspaceColors.indicator": { "extractionState": "manual", "localizations": { @@ -10776,50 +10721,6 @@ } } }, - "settings.workspaceColors.remove": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Remove" - } - } - } - }, - "settings.workspaceColors.resetPalette": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Reset Palette" - } - } - } - }, - "settings.workspaceColors.resetPalette.button": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Reset" - } - } - } - }, - "settings.workspaceColors.resetPalette.subtitleV2": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Restore the built-in palette and remove extra named colors." - } - } - } - }, "shortcut.closeWindow.label": { "extractionState": "manual", "localizations": { diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index 3990f549..19607ae1 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -92,7 +92,6 @@ struct SettingsView: View { @State private var notificationCustomSoundStatusIsError = false @State private var showNotificationCustomSoundErrorAlert = false @State private var notificationCustomSoundErrorAlertMessage = "" - @State private var workspaceTabPaletteEntries = WorkspaceTabColorSettings.palette() @State private var trustedDirectoriesDraft: String = ProgramaDirectoryTrust.shared.allTrustedPaths.joined(separator: "\n") @State private var mobileBridgePairedDevices: [MobileBridgeTrustedDevice] = [] @State private var mobileBridgePairingTicket: String? @@ -616,7 +615,6 @@ struct SettingsView: View { browserHistoryEntryCount = BrowserHistoryStore.shared.entries.count browserInsecureHTTPAllowlistDraft = browserInsecureHTTPAllowlist refreshDetectedImportBrowsers() - reloadWorkspaceTabColorSettings() refreshNotificationCustomSoundStatus() Task { await refreshMobileBridgePairedDevices() } } @@ -635,9 +633,6 @@ struct SettingsView: View { .onReceive(BrowserHistoryStore.shared.$entries) { entries in browserHistoryEntryCount = entries.count } - .onReceive(NotificationCenter.default.publisher(for: UserDefaults.didChangeNotification)) { _ in - reloadWorkspaceTabColorSettings() - } .onReceive(NotificationCenter.default.publisher(for: SettingsNavigationRequest.notificationName)) { notification in guard let target = SettingsNavigationRequest.target(from: notification) else { return } DispatchQueue.main.async { @@ -987,136 +982,13 @@ struct SettingsView: View { Text(style.displayName).tag(style.rawValue) } } - - SettingsCardDivider() - - SettingsCardRow( - String(localized: "settings.workspaceColors.selectionColor", defaultValue: "Selection Highlight"), - subtitle: String(localized: "settings.workspaceColors.selectionColor.subtitle", defaultValue: "Background color of the selected workspace in the sidebar.") - ) { - HStack(spacing: 8) { - if sidebarSelectionColorHex != nil { - Button(String(localized: "settings.workspaceColors.selectionColor.reset", defaultValue: "Reset")) { - sidebarSelectionColorHex = nil - } - .buttonStyle(.bordered) - .controlSize(.small) - } - - HexColorPicker( - hex: sidebarSelectionColorHex, - fallback: programaAccentColor() - ) { newHex in - sidebarSelectionColorHex = newHex - } - - Text(sidebarSelectionColorHex ?? String(localized: "settings.sidebarAppearance.defaultLabel", defaultValue: "Default")) - .font(.system(size: 12, weight: .medium, design: .monospaced)) - .foregroundStyle(.secondary) - .frame(width: 76, alignment: .trailing) - } - } - - SettingsCardDivider() - - SettingsCardRow( - String(localized: "settings.workspaceColors.notificationBadgeColor", defaultValue: "Notification Badge"), - subtitle: String(localized: "settings.workspaceColors.notificationBadgeColor.subtitle", defaultValue: "Color of the unread notification badge on workspace tabs.") - ) { - HStack(spacing: 8) { - if sidebarNotificationBadgeColorHex != nil { - Button(String(localized: "settings.workspaceColors.notificationBadgeColor.reset", defaultValue: "Reset")) { - sidebarNotificationBadgeColorHex = nil - } - .buttonStyle(.bordered) - .controlSize(.small) - } - - HexColorPicker( - hex: sidebarNotificationBadgeColorHex, - fallback: programaAccentColor() - ) { newHex in - sidebarNotificationBadgeColorHex = newHex - } - - Text(sidebarNotificationBadgeColorHex ?? String(localized: "settings.sidebarAppearance.defaultLabel", defaultValue: "Default")) - .font(.system(size: 12, weight: .medium, design: .monospaced)) - .foregroundStyle(.secondary) - .frame(width: 76, alignment: .trailing) - } - } - - SettingsCardDivider() - - SettingsCardNote( - String( - localized: "settings.workspaceColors.dictionaryNote", - defaultValue: "Edit settings.json to add or remove named colors. \"Choose Custom Color...\" still adds local Custom N entries." - ) - ) - - if workspaceTabPaletteEntries.isEmpty { - SettingsCardNote( - String( - localized: "settings.workspaceColors.emptyPalette", - defaultValue: "No palette entries. Add colors in settings.json or use \"Choose Custom Color...\" from a workspace context menu." - ) - ) - } else { - ForEach(Array(workspaceTabPaletteEntries.enumerated()), id: \.element.name) { index, entry in - if index > 0 { - SettingsCardDivider() - } - SettingsCardRow( - entry.name, - subtitle: baseTabColorHex(for: entry.name).map { - String(localized: "settings.workspaceColors.base", defaultValue: "Base: \($0)") - } ?? String( - localized: "settings.workspaceColors.customEntry", - defaultValue: "Named palette entry." - ) - ) { - HStack(spacing: 8) { - HexColorPicker( - hex: entry.hex, - fallback: .blue - ) { newHex in - WorkspaceTabColorSettings.setColor(named: entry.name, hex: newHex) - reloadWorkspaceTabColorSettings() - } - - Text(entry.hex) - .font(.system(size: 12, weight: .medium, design: .monospaced)) - .foregroundStyle(.secondary) - .frame(width: 76, alignment: .trailing) - - if baseTabColorHex(for: entry.name) == nil { - Button(String(localized: "settings.workspaceColors.remove", defaultValue: "Remove")) { - removeWorkspaceColor(named: entry.name) - } - .buttonStyle(.bordered) - .controlSize(.small) - } - } - } - } - } - - SettingsCardDivider() - - SettingsCardRow( - String(localized: "settings.workspaceColors.resetPalette", defaultValue: "Reset Palette"), - subtitle: String( - localized: "settings.workspaceColors.resetPalette.subtitleV2", - defaultValue: "Restore the built-in palette and remove extra named colors." - ) - ) { - Button(String(localized: "settings.workspaceColors.resetPalette.button", defaultValue: "Reset")) { - resetWorkspaceTabColors() - } - .buttonStyle(.bordered) - .controlSize(.small) - } + // Selection-highlight and notification-badge hex pickers, the inline + // palette editor and a section-local Reset Palette used to follow. The + // editor duplicated settings.json, which its own note already pointed at + // as the way to manage named colors, and the two hex pickers were the + // same per-pixel tuning as the sidebar tints. "Reset all settings" still + // calls WorkspaceTabColorSettings.reset() and nils both hex keys, so + // anything set while these rows existed is still recoverable. } } @@ -1964,28 +1836,9 @@ struct SettingsView: View { refreshDetectedImportBrowsers() KeyboardShortcutSettings.resetAll() WorkspaceTabColorSettings.reset() - reloadWorkspaceTabColorSettings() shortcutResetToken = UUID() } - private func baseTabColorHex(for name: String) -> String? { - WorkspaceTabColorSettings.defaultColorHex(named: name) - } - - private func removeWorkspaceColor(named name: String) { - WorkspaceTabColorSettings.removeColor(named: name) - reloadWorkspaceTabColorSettings() - } - - private func resetWorkspaceTabColors() { - WorkspaceTabColorSettings.reset() - reloadWorkspaceTabColorSettings() - } - - private func reloadWorkspaceTabColorSettings() { - workspaceTabPaletteEntries = WorkspaceTabColorSettings.palette() - } - private func saveBrowserInsecureHTTPAllowlist() { browserInsecureHTTPAllowlist = browserInsecureHTTPAllowlistDraft } From b2dd1f698fed564b5ee8b2c94276b7adc84e3896 Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:16:24 -0300 Subject: [PATCH 03/11] fix(settings): localize the two App rows that shipped bare strings "Desktop Notifications" and "Notification Command" passed raw string literals for title, subtitle and placeholder, so they stayed English in a Japanese UI. Every other row in the file uses String(localized:). Found while inventorying the App section for the settings cut. --- Resources/Localizable.xcstrings | 218 +++++++++++++++++++++----------- Sources/SettingsView.swift | 14 +- 2 files changed, 153 insertions(+), 79 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 4b84a809..f14dc19f 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -6903,36 +6903,36 @@ } } }, - "dialog.cmuxConfig.confirmCommand.trustDirectory": { + "dialog.cmuxConfig.confirmCommand.truncated": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Always trust commands from this folder" + "value": "… (truncated)" } }, "ja": { "stringUnit": { "state": "translated", - "value": "このフォルダのコマンドを常に信頼する" + "value": "…(省略)" } } } }, - "dialog.cmuxConfig.confirmCommand.truncated": { + "dialog.cmuxConfig.confirmCommand.trustDirectory": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "… (truncated)" + "value": "Always trust commands from this folder" } }, "ja": { "stringUnit": { "state": "translated", - "value": "…(省略)" + "value": "このフォルダのコマンドを常に信頼する" } } } @@ -9789,284 +9789,275 @@ } } }, - "settings.notifications.longCommandThreshold.subtitle": { + "settings.notifications.command.placeholder": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Notify when a command finishes in a pane you're not looking at, if it ran at least this long. Set to 0 to disable." + "value": "say \"done\"" } }, "ja": { "stringUnit": { "state": "translated", - "value": "見ていないペインでコマンドが完了したとき、実行時間がこの秒数以上であれば通知します。0にすると無効になります。" + "value": "say \"done\"" } } } }, - "settings.notifications.longCommandThreshold.title": { + "settings.notifications.command.subtitle": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Long Command Notification" + "value": "Run a shell command when a notification arrives. $PROGRAMA_NOTIFICATION_TITLE, $PROGRAMA_NOTIFICATION_SUBTITLE, $PROGRAMA_NOTIFICATION_BODY are set." } }, "ja": { "stringUnit": { "state": "translated", - "value": "長時間コマンドの通知" + "value": "通知が届いたときにシェルコマンドを実行します。$PROGRAMA_NOTIFICATION_TITLE、$PROGRAMA_NOTIFICATION_SUBTITLE、$PROGRAMA_NOTIFICATION_BODY が設定されます。" } } } }, - "settings.notifications.longCommandThreshold.unit": { + "settings.notifications.command.title": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "sec" + "value": "Notification Command" } }, "ja": { "stringUnit": { "state": "translated", - "value": "秒" + "value": "通知コマンド" } } } }, - "settings.notifications.sound.custom.choose.button": { + "settings.notifications.desktop.title": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Choose..." + "value": "Desktop Notifications" } - } - } - }, - "settings.notifications.sound.custom.choose.prompt": { - "extractionState": "manual", - "localizations": { - "en": { + }, + "ja": { "stringUnit": { "state": "translated", - "value": "Choose" + "value": "デスクトップ通知" } } } }, - "settings.notifications.sound.custom.choose.title": { + "settings.notifications.longCommandThreshold.subtitle": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Choose Notification Sound" + "value": "Notify when a command finishes in a pane you're not looking at, if it ran at least this long. Set to 0 to disable." } - } - } - }, - "settings.notifications.sound.custom.clear.button": { - "extractionState": "manual", - "localizations": { - "en": { + }, + "ja": { "stringUnit": { "state": "translated", - "value": "Clear" + "value": "見ていないペインでコマンドが完了したとき、実行時間がこの秒数以上であれば通知します。0にすると無効になります。" } } } }, - "settings.notifications.sound.custom.error.title": { + "settings.notifications.longCommandThreshold.title": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Custom Notification Sound Error" + "value": "Long Command Notification" } - } - } - }, - "settings.notifications.sound.custom.file.none": { - "extractionState": "manual", - "localizations": { - "en": { + }, + "ja": { "stringUnit": { "state": "translated", - "value": "No file selected" + "value": "長時間コマンドの通知" } } } }, - "settings.notifications.sound.custom.status.empty": { + "settings.notifications.longCommandThreshold.unit": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Choose a custom audio file first." + "value": "sec" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "秒" } } } }, - "settings.notifications.sound.custom.status.missingExtensionPrefix": { + "settings.notifications.sound.custom.choose.button": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "File needs an extension: " + "value": "Choose..." } } } }, - "settings.notifications.sound.custom.status.missingFilePrefix": { + "settings.notifications.sound.custom.choose.prompt": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "File not found: " + "value": "Choose" } } } }, - "settings.notifications.sound.custom.status.prepareFailed": { + "settings.notifications.sound.custom.choose.title": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Could not prepare this file for notifications. Try WAV, AIFF, or CAF." + "value": "Choose Notification Sound" } } } }, - "settings.notifications.sound.custom.status.ready": { + "settings.notifications.sound.custom.clear.button": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Ready for notifications." + "value": "Clear" } } } }, - "settings.notifications.sound.custom.status.readyConverted": { + "settings.notifications.sound.custom.error.title": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Prepared for notifications (converted to CAF)." + "value": "Custom Notification Sound Error" } } } }, - "settings.notifications.sound.subtitle": { + "settings.notifications.sound.custom.file.none": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Sound played when a notification arrives." + "value": "No file selected" } } } }, - "settings.notifications.sound.title": { + "settings.notifications.sound.custom.status.empty": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Notification Sound" + "value": "Choose a custom audio file first." } } } }, - "settings.preset.hudGlass": { + "settings.notifications.sound.custom.status.missingExtensionPrefix": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "HUD Glass" + "value": "File needs an extension: " } } } }, - "settings.preset.nativeSidebar": { + "settings.notifications.sound.custom.status.missingFilePrefix": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Native Sidebar" + "value": "File not found: " } } } }, - "settings.preset.popoverGlass": { + "settings.notifications.sound.custom.status.prepareFailed": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Popover Glass" + "value": "Could not prepare this file for notifications. Try WAV, AIFF, or CAF." } } } }, - "settings.preset.raycastGray": { + "settings.notifications.sound.custom.status.ready": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Raycast Gray" + "value": "Ready for notifications." } } } }, - "settings.preset.softBlur": { + "settings.notifications.sound.custom.status.readyConverted": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Soft Blur" + "value": "Prepared for notifications (converted to CAF)." } } } }, - "settings.preset.underWindow": { + "settings.notifications.sound.subtitle": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Under Window" + "value": "Sound played when a notification arrives." } } } }, - "settings.reset.resetAll": { + "settings.notifications.sound.title": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Reset All Settings" + "value": "Notification Sound" } } } @@ -10428,6 +10419,83 @@ } } }, + "settings.preset.hudGlass": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "HUD Glass" + } + } + } + }, + "settings.preset.nativeSidebar": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Native Sidebar" + } + } + } + }, + "settings.preset.popoverGlass": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Popover Glass" + } + } + } + }, + "settings.preset.raycastGray": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Raycast Gray" + } + } + } + }, + "settings.preset.softBlur": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Soft Blur" + } + } + } + }, + "settings.preset.underWindow": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Under Window" + } + } + } + }, + "settings.reset.resetAll": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Reset All Settings" + } + } + } + }, "settings.section.app": { "extractionState": "manual", "localizations": { diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index 19607ae1..5463ebe1 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -790,7 +790,7 @@ struct SettingsView: View { SettingsCardDivider() SettingsCardRow( - "Desktop Notifications", + String(localized: "settings.notifications.desktop.title", defaultValue: "Desktop Notifications"), subtitle: notificationPermissionSubtitle ) { HStack(spacing: 6) { @@ -877,10 +877,16 @@ struct SettingsView: View { SettingsCardDivider() SettingsCardRow( - "Notification Command", - subtitle: "Run a shell command when a notification arrives. $PROGRAMA_NOTIFICATION_TITLE, $PROGRAMA_NOTIFICATION_SUBTITLE, $PROGRAMA_NOTIFICATION_BODY are set." + String(localized: "settings.notifications.command.title", defaultValue: "Notification Command"), + subtitle: String( + localized: "settings.notifications.command.subtitle", + defaultValue: "Run a shell command when a notification arrives. $PROGRAMA_NOTIFICATION_TITLE, $PROGRAMA_NOTIFICATION_SUBTITLE, $PROGRAMA_NOTIFICATION_BODY are set." + ) ) { - TextField("say \"done\"", text: $notificationCustomCommand) + TextField( + String(localized: "settings.notifications.command.placeholder", defaultValue: "say \"done\""), + text: $notificationCustomCommand + ) .textFieldStyle(.roundedBorder) .frame(width: 200) } From 29ed091873bcf5b5cdb206b71c619ee052b2d25b Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:24:26 -0300 Subject: [PATCH 04/11] feat(settings): split the settings window into six tabs Settings was one scroll of nine stacked sections, so finding anything meant knowing roughly how far down it lived. Sections that answer the same question are merged: Workspace Colors and Sidebar Appearance become Appearance, Custom Commands joins Automation, and Reset sits with the general app switches it resets. That gives General, Appearance, Automation, Phone, Browser. Keyboard Shortcuts stays on its own. It renders one row per KeyboardShortcutSettings.Action, and there are 57 of them, so it would swamp whatever it shared a tab with. The tab strip lives in the header overlay rather than the scroll content, so it stays put while a tab's rows scroll under the existing blur. The header grows from 62pt to 96pt and the content inset moves with it. SettingsNavigationRequest deep links still work: the handler selects the owning tab before scrolling, because an anchor only exists while its tab is selected. browserImport is the one that still needs the scroll, since it sits mid-tab rather than at the top. --- Resources/Localizable.xcstrings | 102 ++++++++++++++++++++++++++++++++ Sources/SettingsModels.swift | 42 +++++++++++++ Sources/SettingsView.swift | 82 +++++++++++++++++-------- 3 files changed, 201 insertions(+), 25 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index f14dc19f..0c596385 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -10767,6 +10767,108 @@ } } }, + "settings.tab.appearance": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Appearance" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "外観" + } + } + } + }, + "settings.tab.automation": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Automation" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "自動化" + } + } + } + }, + "settings.tab.browser": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Browser" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "ブラウザ" + } + } + } + }, + "settings.tab.general": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "General" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "一般" + } + } + } + }, + "settings.tab.phone": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Phone" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "スマートフォン" + } + } + } + }, + "settings.tab.shortcuts": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Shortcuts" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "ショートカット" + } + } + } + }, "settings.title": { "extractionState": "manual", "localizations": { diff --git a/Sources/SettingsModels.swift b/Sources/SettingsModels.swift index dcfdd8e2..22597033 100644 --- a/Sources/SettingsModels.swift +++ b/Sources/SettingsModels.swift @@ -222,3 +222,45 @@ enum PreferredEditorSettings { } } + +/// The tabs the settings window is split across. +/// +/// Settings used to be one scroll of nine stacked sections, which meant finding +/// anything required knowing roughly how far down it lived. The grouping here +/// merges sections that answer the same question: Workspace Colors and Sidebar +/// Appearance are both "how it looks", Custom Commands is part of Automation, +/// and Reset belongs with the general app switches it resets. +/// +/// Keyboard Shortcuts stays on its own because it renders one row per +/// `KeyboardShortcutSettings.Action` -- 57 of them -- and would swamp whatever +/// it shared a tab with. +enum SettingsTab: String, CaseIterable, Identifiable { + case general + case appearance + case automation + case phone + case browser + case shortcuts + + var id: String { rawValue } + + var title: String { + switch self { + case .general: String(localized: "settings.tab.general", defaultValue: "General") + case .appearance: String(localized: "settings.tab.appearance", defaultValue: "Appearance") + case .automation: String(localized: "settings.tab.automation", defaultValue: "Automation") + case .phone: String(localized: "settings.tab.phone", defaultValue: "Phone") + case .browser: String(localized: "settings.tab.browser", defaultValue: "Browser") + case .shortcuts: String(localized: "settings.tab.shortcuts", defaultValue: "Shortcuts") + } + } + + /// The tab that owns a deep-link target, so `SettingsNavigationRequest` + /// still lands on the right content now that it is not all one scroll. + static func owning(_ target: SettingsNavigationTarget) -> SettingsTab { + switch target { + case .browser, .browserImport: .browser + case .keyboardShortcuts: .shortcuts + } + } +} diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index 5463ebe1..ee3e0fe3 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -8,7 +8,12 @@ import CoreImage.CIFilterBuiltins struct SettingsView: View { - private let contentTopInset: CGFloat = 8 + // The header overlay now carries a tab strip under the title, so content has + // to start below both rather than just below the title. + private let contentTopInset: CGFloat = 42 + private let headerHeight: CGFloat = 96 + + @State private var selectedTab: SettingsTab = .general private let pickerColumnWidth: CGFloat = 196 private let notificationSoundControlWidth: CGFloat = 280 private let shortcutChordsDocsURL = URL(string: "https://github.com/darkroomengineering/programa/tree/main/docs")! @@ -514,15 +519,23 @@ struct SettingsView: View { ZStack(alignment: .top) { ScrollView { VStack(alignment: .leading, spacing: 14) { - appSection - workspaceColorsSection - sidebarAppearanceSection - automationSection - phoneSection - customCommandsSection - browserSection - keyboardShortcutsSection - resetSection + switch selectedTab { + case .general: + appSection + resetSection + case .appearance: + workspaceColorsSection + sidebarAppearanceSection + case .automation: + automationSection + customCommandsSection + case .phone: + phoneSection + case .browser: + browserSection + case .shortcuts: + keyboardShortcutsSection + } } .padding(.horizontal, 20) .padding(.bottom, 20) @@ -578,25 +591,41 @@ struct SettingsView: View { ) .opacity(0.14 + (topBlurOpacity * 0.86)) - HStack { - Text(String(localized: "settings.title", defaultValue: "Settings")) - .font(.system(size: 16, weight: .semibold)) - .foregroundColor(.primary.opacity(0.92)) - Spacer(minLength: 0) - HStack(spacing: 6) { - SettingsHeaderActionButton( - title: String(localized: "settings.app.settingsFile.openButton", defaultValue: "Open settings.json"), - helpText: KeyboardShortcutSettings.settingsFileStore.settingsFileDisplayPath(), - accessibilityIdentifier: "SettingsFileOpenButton", - action: openProgramaSettingsFileInTextEdit - ) + VStack(spacing: 8) { + HStack { + Text(String(localized: "settings.title", defaultValue: "Settings")) + .font(.system(size: 16, weight: .semibold)) + .foregroundColor(.primary.opacity(0.92)) + Spacer(minLength: 0) + HStack(spacing: 6) { + SettingsHeaderActionButton( + title: String(localized: "settings.app.settingsFile.openButton", defaultValue: "Open settings.json"), + helpText: KeyboardShortcutSettings.settingsFileStore.settingsFileDisplayPath(), + accessibilityIdentifier: "SettingsFileOpenButton", + action: openProgramaSettingsFileInTextEdit + ) + } + } + .padding(.leading, settingsTitleLeadingInset) + .padding(.trailing, 20) + + // Lives in the header overlay rather than the scroll content so + // it stays put while a tab's rows scroll under the blur. + Picker("", selection: $selectedTab) { + ForEach(SettingsTab.allCases) { tab in + Text(tab.title).tag(tab) + } } + .pickerStyle(.segmented) + .labelsHidden() + .controlSize(.small) + .padding(.leading, settingsTitleLeadingInset) + .padding(.trailing, 20) + .accessibilityIdentifier("SettingsTabPicker") } - .padding(.leading, settingsTitleLeadingInset) - .padding(.trailing, 20) .padding(.top, 12) } - .frame(height: 62) + .frame(height: headerHeight) .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) .ignoresSafeArea(.container, edges: .top) .overlay( @@ -635,6 +664,9 @@ struct SettingsView: View { } .onReceive(NotificationCenter.default.publisher(for: SettingsNavigationRequest.notificationName)) { notification in guard let target = SettingsNavigationRequest.target(from: notification) else { return } + // Select the owning tab first. The anchor only exists while its tab is + // the selected one, so scrolling in the same pass would find nothing. + selectedTab = SettingsTab.owning(target) DispatchQueue.main.async { withAnimation(.easeInOut(duration: 0.2)) { proxy.scrollTo(target, anchor: .top) From c8d47f92fe1e81d7708739488b15aab524bd9ff6 Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:31:49 -0300 Subject: [PATCH 05/11] refactor(settings): delete the keep-workspace-open-on-last-surface toggle Cmd+W on a workspace's last surface now always closes the workspace, which was already the default. The opt-out is gone. Removed end to end rather than just hidden: the row and its inverted binding in SettingsView, the LastSurfaceCloseShortcutSettings enum in TabManager, both the parse and emit sides of keepWorkspaceOpenWhenClosingLastSurface in ProgramaSettingsFileStore, and three localization keys. An existing settings.json that still sets keepWorkspaceOpenWhenClosingLastSurface is harmless: the store only reads keys it knows about, so the entry is ignored rather than rejected. shouldCloseWorkspaceOnLastSurfaceShortcut keeps its other two conditions, so the close button still routes through Workspace's explicit-close path. --- Resources/Localizable.xcstrings | 33 --------------------- Sources/ProgramaSettingsFileStore.swift | 4 --- Sources/SettingsView.swift | 38 ------------------------- Sources/TabManager.swift | 21 ++------------ 4 files changed, 3 insertions(+), 93 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 0c596385..97aeabb0 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -8515,39 +8515,6 @@ } } }, - "settings.app.closeWorkspaceOnLastSurfaceShortcut": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Keep Workspace Open When Closing Last Surface" - } - } - } - }, - "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOff": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "When the focused surface is the last one in its workspace, the close-surface shortcut also closes the workspace." - } - } - } - }, - "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOn": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "When the focused surface is the last one in its workspace, the close-surface shortcut closes only the surface and keeps the workspace open. Use the close-workspace shortcut to close the workspace explicitly." - } - } - } - }, "settings.app.commandPaletteSearchAllSurfaces": { "extractionState": "manual", "localizations": { diff --git a/Sources/ProgramaSettingsFileStore.swift b/Sources/ProgramaSettingsFileStore.swift index c30637c0..7b448c8e 100644 --- a/Sources/ProgramaSettingsFileStore.swift +++ b/Sources/ProgramaSettingsFileStore.swift @@ -339,9 +339,6 @@ final class ProgramaSettingsFileStore { let mode = value ? WorkspacePresentationModeSettings.Mode.minimal : .standard snapshot.managedUserDefaults[WorkspacePresentationModeSettings.modeKey] = .string(mode.rawValue) } - if let value = jsonBool(section["keepWorkspaceOpenWhenClosingLastSurface"]) { - snapshot.managedUserDefaults[LastSurfaceCloseShortcutSettings.key] = .bool(!value) - } if let value = jsonBool(section["focusPaneOnFirstClick"]) { snapshot.managedUserDefaults[PaneFirstClickFocusSettings.enabledKey] = .bool(value) } @@ -1210,7 +1207,6 @@ final class ProgramaSettingsFileStore { "appearance": AppearanceSettings.defaultMode.rawValue, "newWorkspacePlacement": WorkspacePlacementSettings.defaultPlacement.rawValue, "minimalMode": WorkspacePresentationModeSettings.defaultMode == .minimal, - "keepWorkspaceOpenWhenClosingLastSurface": !LastSurfaceCloseShortcutSettings.defaultValue, "focusPaneOnFirstClick": PaneFirstClickFocusSettings.defaultEnabled, "preferredEditor": "", "reorderOnNotification": WorkspaceAutoReorderSettings.defaultValue, diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index ee3e0fe3..d235b1ad 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -60,8 +60,6 @@ struct SettingsView: View { @AppStorage(ShortcutHintDebugSettings.alwaysShowHintsKey) private var alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints @AppStorage(WorkspacePlacementSettings.placementKey) private var newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue - @AppStorage(LastSurfaceCloseShortcutSettings.key) - private var closeWorkspaceOnLastSurfaceShortcut = LastSurfaceCloseShortcutSettings.defaultValue @AppStorage(PaneFirstClickFocusSettings.enabledKey) private var paneFirstClickFocusEnabled = PaneFirstClickFocusSettings.defaultEnabled @AppStorage(WorkspaceAutoReorderSettings.key) private var workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue @@ -126,30 +124,6 @@ struct SettingsView: View { ) } - private var keepWorkspaceOpenOnLastSurfaceShortcut: Bool { - !closeWorkspaceOnLastSurfaceShortcut - } - - private var keepWorkspaceOpenOnLastSurfaceShortcutBinding: Binding { - Binding( - get: { keepWorkspaceOpenOnLastSurfaceShortcut }, - set: { closeWorkspaceOnLastSurfaceShortcut = !$0 } - ) - } - - private var closeWorkspaceOnLastSurfaceShortcutSubtitle: String { - if keepWorkspaceOpenOnLastSurfaceShortcut { - return String( - localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOn", - defaultValue: "When the focused surface is the last one in its workspace, the close-surface shortcut closes only the surface and keeps the workspace open. Use the close-workspace shortcut to close the workspace explicitly." - ) - } - return String( - localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut.subtitleOff", - defaultValue: "When the focused surface is the last one in its workspace, the close-surface shortcut also closes the workspace." - ) - } - private var paneFirstClickFocusSubtitle: String { if paneFirstClickFocusEnabled { return String( @@ -755,17 +729,6 @@ struct SettingsView: View { SettingsCardDivider() - SettingsCardRow( - String(localized: "settings.app.closeWorkspaceOnLastSurfaceShortcut", defaultValue: "Keep Workspace Open When Closing Last Surface"), - subtitle: closeWorkspaceOnLastSurfaceShortcutSubtitle - ) { - Toggle("", isOn: keepWorkspaceOpenOnLastSurfaceShortcutBinding) - .labelsHidden() - .controlSize(.small) - } - - SettingsCardDivider() - SettingsCardRow( String(localized: "settings.app.paneFirstClickFocus", defaultValue: "Focus Pane on First Click"), subtitle: paneFirstClickFocusSubtitle @@ -1853,7 +1816,6 @@ struct SettingsView: View { alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue workspacePresentationMode = WorkspacePresentationModeSettings.defaultMode.rawValue - closeWorkspaceOnLastSurfaceShortcut = LastSurfaceCloseShortcutSettings.defaultValue paneFirstClickFocusEnabled = PaneFirstClickFocusSettings.defaultEnabled workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue sidebarActiveTabIndicatorStyle = SidebarActiveTabIndicatorSettings.defaultStyle.rawValue diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 731da39d..c0731211 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -56,20 +56,6 @@ enum WorkspaceAutoReorderSettings { } } -enum LastSurfaceCloseShortcutSettings { - static let key = "closeWorkspaceOnLastSurfaceShortcut" - // Keep the legacy stored meaning so existing values still map to the same - // behavior. The default is flipped to preserve current Cmd+W behavior. - static let defaultValue = true - - static func closesWorkspace(defaults: UserDefaults = .standard) -> Bool { - if defaults.object(forKey: key) == nil { - return defaultValue - } - return defaults.bool(forKey: key) - } -} - struct SidebarWorkspaceAuxiliaryDetailVisibility: Equatable { let showsMetadata: Bool let showsLog: Bool @@ -2232,8 +2218,7 @@ class TabManager: ObservableObject { } private func shouldCloseWorkspaceOnLastSurfaceShortcut(_ workspace: Workspace, panelId: UUID) -> Bool { - LastSurfaceCloseShortcutSettings.closesWorkspace() && - workspace.panels.count <= 1 && + workspace.panels.count <= 1 && workspace.panels[panelId] != nil } @@ -2267,8 +2252,8 @@ class TabManager: ObservableObject { ) #endif - // The last-surface shortcut preference only affects Cmd+W. The tab close button - // continues to use Workspace's explicit-close path when it closes the last surface. + // Only Cmd+W closes the workspace along with its last surface. The tab close + // button continues to use Workspace's explicit-close path instead. if closesWorkspaceOnLastSurfaceShortcut, let surfaceId = tab.surfaceIdFromPanelId(panelId) { tab.markExplicitClose(surfaceId: surfaceId) From 57ded5c9a415a9ed1537da00debbf18eb4b3bbba Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:34:48 -0300 Subject: [PATCH 06/11] refactor(settings): delete the focus-pane-on-first-click toggle Clicking into an inactive Programa window now always activates the window without also focusing the pane under the pointer, which was already the default. The opt-in is gone. Every read site was gated on a flag that is now permanently false, so the gates go with it: - GhosttyNSView and ProgramaWebView lose their acceptsFirstMouse overrides entirely. NSView already returns false, so the overrides were restating the default and reading UserDefaults on a mouse-down path to do it. - MarkdownPanelPointerObserverView keeps its hitTest override, now returning nil unconditionally. Deleting it would have fallen back to NSView's default hit-testing, which is the opposite of what the view did. - Its acceptsFirstMouse override goes, and the dead branch in shouldHandle goes with it. Also removes the PaneFirstClickFocusSettings enum, both schema sides of focusPaneOnFirstClick, and three localization keys. InactivePaneFirstClickFocusTests now pins the single behaviour instead of exercising both sides of a toggle, including that writing the old defaults key no longer re-enables click-through. That last one is the real regression guard: a live read left behind would make the removal cosmetic for anyone whose settings.json still carries the key. Also drops LastSurfaceCloseShortcutSettingsTests, which covered the enum deleted in the previous commit. That commit only built the app target and missed it. --- Resources/Localizable.xcstrings | 33 --------- .../GhosttyTerminalView+Accessibility.swift | 4 -- Sources/Panels/MarkdownPanelView.swift | 16 +---- Sources/Panels/ProgramaWebView.swift | 4 -- Sources/ProgramaSettingsFileStore.swift | 4 -- Sources/SettingsModels.swift | 10 --- Sources/SettingsView.swift | 30 -------- .../InactivePaneFirstClickFocusTests.swift | 68 +++++++++---------- .../ShortcutAndCommandPaletteTests.swift | 38 ----------- 9 files changed, 34 insertions(+), 173 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 97aeabb0..6e6d9d39 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -8592,39 +8592,6 @@ } } }, - "settings.app.paneFirstClickFocus": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Focus Pane on First Click" - } - } - } - }, - "settings.app.paneFirstClickFocus.subtitleOff": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "When Programa is inactive, the first click only activates the window. Click again to focus the pane." - } - } - } - }, - "settings.app.paneFirstClickFocus.subtitleOn": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "When Programa is inactive, clicking a pane activates the window and focuses that pane in one click." - } - } - } - }, "settings.app.persistScrollback": { "extractionState": "manual", "localizations": { diff --git a/Sources/GhosttyTerminalView+Accessibility.swift b/Sources/GhosttyTerminalView+Accessibility.swift index 4b086836..690b0bed 100644 --- a/Sources/GhosttyTerminalView+Accessibility.swift +++ b/Sources/GhosttyTerminalView+Accessibility.swift @@ -126,10 +126,6 @@ extension GhosttyNSView { _ = inputContext.perform(updateSelectionSelector) } - override func acceptsFirstMouse(for event: NSEvent?) -> Bool { - PaneFirstClickFocusSettings.isEnabled() - } - override var acceptsFirstResponder: Bool { true } override func becomeFirstResponder() -> Bool { diff --git a/Sources/Panels/MarkdownPanelView.swift b/Sources/Panels/MarkdownPanelView.swift index 71ce8f44..382307d9 100644 --- a/Sources/Panels/MarkdownPanelView.swift +++ b/Sources/Panels/MarkdownPanelView.swift @@ -339,16 +339,9 @@ final class MarkdownPanelPointerObserverView: NSView { } } - override func hitTest(_ point: NSPoint) -> NSView? { - guard PaneFirstClickFocusSettings.isEnabled(), - window?.isKeyWindow != true, - bounds.contains(point) else { return nil } - return self - } - - override func acceptsFirstMouse(for event: NSEvent?) -> Bool { - PaneFirstClickFocusSettings.isEnabled() - } + // This overlay never takes part in hit-testing. It only ever did so while + // the first-click-focus preference was on, and that preference is gone. + override func hitTest(_ point: NSPoint) -> NSView? { nil } override func mouseDown(with event: NSEvent) { onPointerDown?() @@ -370,9 +363,6 @@ final class MarkdownPanelPointerObserverView: NSView { let window, event.window === window, !isHiddenOrHasHiddenAncestor else { return false } - if PaneFirstClickFocusSettings.isEnabled(), window.isKeyWindow != true { - return false - } let point = convert(event.locationInWindow, from: nil) return bounds.contains(point) } diff --git a/Sources/Panels/ProgramaWebView.swift b/Sources/Panels/ProgramaWebView.swift index b4b810af..1101632c 100644 --- a/Sources/Panels/ProgramaWebView.swift +++ b/Sources/Panels/ProgramaWebView.swift @@ -175,10 +175,6 @@ final class ProgramaWebView: WKWebView { } var debugPointerFocusAllowanceDepth: Int { pointerFocusAllowanceDepth } - override func acceptsFirstMouse(for event: NSEvent?) -> Bool { - PaneFirstClickFocusSettings.isEnabled() - } - override func becomeFirstResponder() -> Bool { guard allowsFirstResponderAcquisitionEffective else { #if DEBUG diff --git a/Sources/ProgramaSettingsFileStore.swift b/Sources/ProgramaSettingsFileStore.swift index 7b448c8e..3ddc7f9e 100644 --- a/Sources/ProgramaSettingsFileStore.swift +++ b/Sources/ProgramaSettingsFileStore.swift @@ -339,9 +339,6 @@ final class ProgramaSettingsFileStore { let mode = value ? WorkspacePresentationModeSettings.Mode.minimal : .standard snapshot.managedUserDefaults[WorkspacePresentationModeSettings.modeKey] = .string(mode.rawValue) } - if let value = jsonBool(section["focusPaneOnFirstClick"]) { - snapshot.managedUserDefaults[PaneFirstClickFocusSettings.enabledKey] = .bool(value) - } if let value = jsonString(section["preferredEditor"]) { snapshot.managedUserDefaults[PreferredEditorSettings.key] = .string(value) } @@ -1207,7 +1204,6 @@ final class ProgramaSettingsFileStore { "appearance": AppearanceSettings.defaultMode.rawValue, "newWorkspacePlacement": WorkspacePlacementSettings.defaultPlacement.rawValue, "minimalMode": WorkspacePresentationModeSettings.defaultMode == .minimal, - "focusPaneOnFirstClick": PaneFirstClickFocusSettings.defaultEnabled, "preferredEditor": "", "reorderOnNotification": WorkspaceAutoReorderSettings.defaultValue, "warnBeforeQuit": QuitWarningSettings.defaultWarnBeforeQuit, diff --git a/Sources/SettingsModels.swift b/Sources/SettingsModels.swift index 22597033..b2e78cf6 100644 --- a/Sources/SettingsModels.swift +++ b/Sources/SettingsModels.swift @@ -28,16 +28,6 @@ enum WorkspacePresentationModeSettings { } } -enum PaneFirstClickFocusSettings { - static let enabledKey = "paneFirstClickFocus.enabled" - static let defaultEnabled = false - private static let flag = UserDefaultsFlag(key: enabledKey, defaultValue: defaultEnabled) - - static func isEnabled(defaults: UserDefaults = .standard) -> Bool { - flag.isEnabled(defaults: defaults) - } -} - enum AppearanceMode: String, CaseIterable, Identifiable { case system case light diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index d235b1ad..e4092fa5 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -60,8 +60,6 @@ struct SettingsView: View { @AppStorage(ShortcutHintDebugSettings.alwaysShowHintsKey) private var alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints @AppStorage(WorkspacePlacementSettings.placementKey) private var newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue - @AppStorage(PaneFirstClickFocusSettings.enabledKey) - private var paneFirstClickFocusEnabled = PaneFirstClickFocusSettings.defaultEnabled @AppStorage(WorkspaceAutoReorderSettings.key) private var workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue @AppStorage(SidebarActiveTabIndicatorSettings.styleKey) private var sidebarActiveTabIndicatorStyle = SidebarActiveTabIndicatorSettings.defaultStyle.rawValue @@ -124,19 +122,6 @@ struct SettingsView: View { ) } - private var paneFirstClickFocusSubtitle: String { - if paneFirstClickFocusEnabled { - return String( - localized: "settings.app.paneFirstClickFocus.subtitleOn", - defaultValue: "When Programa is inactive, clicking a pane activates the window and focuses that pane in one click." - ) - } - return String( - localized: "settings.app.paneFirstClickFocus.subtitleOff", - defaultValue: "When Programa is inactive, the first click only activates the window. Click again to focus the pane." - ) - } - private var selectedSidebarActiveTabIndicatorStyle: SidebarActiveTabIndicatorStyle { SidebarActiveTabIndicatorSettings.resolvedStyle(rawValue: sidebarActiveTabIndicatorStyle) } @@ -729,20 +714,6 @@ struct SettingsView: View { SettingsCardDivider() - SettingsCardRow( - String(localized: "settings.app.paneFirstClickFocus", defaultValue: "Focus Pane on First Click"), - subtitle: paneFirstClickFocusSubtitle - ) { - Toggle("", isOn: $paneFirstClickFocusEnabled) - .labelsHidden() - .controlSize(.small) - .accessibilityLabel( - String(localized: "settings.app.paneFirstClickFocus", defaultValue: "Focus Pane on First Click") - ) - } - - SettingsCardDivider() - SettingsCardRow( String(localized: "settings.app.preferredEditor", defaultValue: "Open Files With"), subtitle: String(localized: "settings.app.preferredEditor.subtitle", defaultValue: "Command to open files on Cmd-click. Leave empty for system default.") @@ -1816,7 +1787,6 @@ struct SettingsView: View { alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue workspacePresentationMode = WorkspacePresentationModeSettings.defaultMode.rawValue - paneFirstClickFocusEnabled = PaneFirstClickFocusSettings.defaultEnabled workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue sidebarActiveTabIndicatorStyle = SidebarActiveTabIndicatorSettings.defaultStyle.rawValue sidebarSelectionColorHex = nil diff --git a/programaTests/InactivePaneFirstClickFocusTests.swift b/programaTests/InactivePaneFirstClickFocusTests.swift index db275ae3..4ee1627b 100644 --- a/programaTests/InactivePaneFirstClickFocusTests.swift +++ b/programaTests/InactivePaneFirstClickFocusTests.swift @@ -8,65 +8,59 @@ import WebKit @testable import Programa #endif +/// Clicking into an inactive Programa window activates the window without also +/// focusing the pane under the pointer. That used to be a preference +/// (`paneFirstClickFocus.enabled`); it is now the only behaviour, so these +/// tests pin it rather than exercising both sides of a toggle. +/// +/// The three view classes are covered separately because each one used to +/// override `acceptsFirstMouse` on its own, and a future override on any of +/// them would silently reintroduce click-through. @MainActor final class InactivePaneFirstClickFocusTests: XCTestCase { - private let settingsKey = "paneFirstClickFocus.enabled" - - override func setUp() { - super.setUp() - UserDefaults.standard.removeObject(forKey: settingsKey) - } + /// Set by nothing now. Asserting the views ignore it is what proves the + /// preference is genuinely gone rather than merely hidden from Settings. + private let removedSettingsKey = "paneFirstClickFocus.enabled" override func tearDown() { - UserDefaults.standard.removeObject(forKey: settingsKey) + UserDefaults.standard.removeObject(forKey: removedSettingsKey) super.tearDown() } - func testTerminalViewAcceptsFirstMouseWhenSettingEnabled() { - UserDefaults.standard.set(true, forKey: settingsKey) - - let view = GhosttyNSView(frame: .zero) - - XCTAssertTrue(view.acceptsFirstMouse(for: nil)) - } - - func testTerminalViewRejectsFirstMouseWhenSettingDisabled() { - UserDefaults.standard.set(false, forKey: settingsKey) - + func testTerminalViewRejectsFirstMouse() { let view = GhosttyNSView(frame: .zero) XCTAssertFalse(view.acceptsFirstMouse(for: nil)) } - func testBrowserViewAcceptsFirstMouseWhenSettingEnabled() { - UserDefaults.standard.set(true, forKey: settingsKey) - - let view = ProgramaWebView(frame: .zero, configuration: WKWebViewConfiguration()) - - XCTAssertTrue(view.acceptsFirstMouse(for: nil)) - } - - func testBrowserViewRejectsFirstMouseWhenSettingDisabled() { - UserDefaults.standard.set(false, forKey: settingsKey) - + func testBrowserViewRejectsFirstMouse() { let view = ProgramaWebView(frame: .zero, configuration: WKWebViewConfiguration()) XCTAssertFalse(view.acceptsFirstMouse(for: nil)) } - func testMarkdownPointerObserverAcceptsFirstMouseWhenSettingEnabled() { - UserDefaults.standard.set(true, forKey: settingsKey) - + func testMarkdownPointerObserverRejectsFirstMouse() { let view = MarkdownPanelPointerObserverView(frame: .zero) - XCTAssertTrue(view.acceptsFirstMouse(for: nil)) + XCTAssertFalse(view.acceptsFirstMouse(for: nil)) } - func testMarkdownPointerObserverRejectsFirstMouseWhenSettingDisabled() { - UserDefaults.standard.set(false, forKey: settingsKey) + func testRemovedPreferenceNoLongerEnablesClickThrough() { + // The old key turning click-through back on is the regression this + // guards: leaving a live read behind would make the setting removal + // cosmetic for anyone whose settings.json still carries it. + UserDefaults.standard.set(true, forKey: removedSettingsKey) + + XCTAssertFalse(GhosttyNSView(frame: .zero).acceptsFirstMouse(for: nil)) + XCTAssertFalse( + ProgramaWebView(frame: .zero, configuration: WKWebViewConfiguration()).acceptsFirstMouse(for: nil) + ) + XCTAssertFalse(MarkdownPanelPointerObserverView(frame: .zero).acceptsFirstMouse(for: nil)) + } - let view = MarkdownPanelPointerObserverView(frame: .zero) + func testMarkdownPointerObserverStaysOutOfHitTesting() { + let view = MarkdownPanelPointerObserverView(frame: NSRect(x: 0, y: 0, width: 100, height: 100)) - XCTAssertFalse(view.acceptsFirstMouse(for: nil)) + XCTAssertNil(view.hitTest(NSPoint(x: 50, y: 50))) } } diff --git a/programaTests/ShortcutAndCommandPaletteTests.swift b/programaTests/ShortcutAndCommandPaletteTests.swift index 8fa9e8af..ee8d4cbd 100644 --- a/programaTests/ShortcutAndCommandPaletteTests.swift +++ b/programaTests/ShortcutAndCommandPaletteTests.swift @@ -1027,44 +1027,6 @@ final class ShortcutHintHorizontalPlannerTests: XCTestCase { } -final class LastSurfaceCloseShortcutSettingsTests: XCTestCase { - func testDefaultClosesWorkspace() { - let suiteName = "LastSurfaceCloseShortcutSettingsTests.Default.\(UUID().uuidString)" - guard let defaults = UserDefaults(suiteName: suiteName) else { - XCTFail("Failed to create isolated UserDefaults suite") - return - } - defer { defaults.removePersistentDomain(forName: suiteName) } - - XCTAssertTrue(LastSurfaceCloseShortcutSettings.closesWorkspace(defaults: defaults)) - } - - func testStoredTrueClosesWorkspace() { - let suiteName = "LastSurfaceCloseShortcutSettingsTests.Enabled.\(UUID().uuidString)" - guard let defaults = UserDefaults(suiteName: suiteName) else { - XCTFail("Failed to create isolated UserDefaults suite") - return - } - defer { defaults.removePersistentDomain(forName: suiteName) } - - defaults.set(true, forKey: LastSurfaceCloseShortcutSettings.key) - XCTAssertTrue(LastSurfaceCloseShortcutSettings.closesWorkspace(defaults: defaults)) - } - - func testStoredFalseKeepsWorkspaceOpen() { - let suiteName = "LastSurfaceCloseShortcutSettingsTests.Disabled.\(UUID().uuidString)" - guard let defaults = UserDefaults(suiteName: suiteName) else { - XCTFail("Failed to create isolated UserDefaults suite") - return - } - defer { defaults.removePersistentDomain(forName: suiteName) } - - defaults.set(false, forKey: LastSurfaceCloseShortcutSettings.key) - XCTAssertFalse(LastSurfaceCloseShortcutSettings.closesWorkspace(defaults: defaults)) - } -} - - final class AppearanceSettingsTests: XCTestCase { func testResolvedModeDefaultsToSystemWhenUnset() { let suiteName = "AppearanceSettingsTests.Default.\(UUID().uuidString)" From e08d401848a53646c4039e9ca12ed5afffde79ae Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:41:48 -0300 Subject: [PATCH 07/11] refactor(settings): delete the rename-selects-existing-name toggle Command-palette rename now always opens with the existing name selected, so typing replaces it. That was already the default. This one reached further than the other cuts because the setting had a v2 socket method behind it so UI tests could flip it: - Sources: the CommandPaletteRenameSelectionSettings enum, the @AppStorage in CommandPaletteController, a dead read/write proxy in ContentView, both schema sides of renameSelectsExistingName, and the settings row. - Socket API: debug.command_palette.rename_input.select_all is gone, along with its catalog entry, its dispatch case, and the two client helpers in tests_v2/cmux.py. It is a debug.* method, not part of the stable surface. - ContentView's rename focus policy now asks for .selectAll directly. Other focus policies still use .caretAtEnd, so that case stays. Test changes, rather than deletions, where the coverage was still worth having. test_command_palette_rename_select_all.py kept its eight stress cycles proving the selection survives interaction with the field, and lost only the half that toggled the setting off. test_command_palette_backspace_go_back.py no longer pins the setting to get a deterministic starting state, because the behaviour is now guaranteed. CommandPaletteRenameSelectionSettingsTests is gone with the enum it covered. Both the app and the unit-test target build. tests_v2 is CI-only and unrun here. --- Resources/Localizable.xcstrings | 33 ----- Sources/CommandPaletteController.swift | 2 - Sources/ContentView.swift | 13 +- Sources/ProgramaSettingsFileStore.swift | 4 - Sources/SettingsModels.swift | 10 -- Sources/SettingsView.swift | 16 --- Sources/TerminalController+Debug.swift | 27 ---- Sources/TerminalController.swift | 2 - Sources/V2CommandCatalog.swift | 1 - .../ShortcutAndCommandPaletteTests.swift | 28 ---- tests_v2/cmux.py | 8 -- .../test_command_palette_backspace_go_back.py | 10 +- .../test_command_palette_rename_select_all.py | 124 +++++------------- 13 files changed, 42 insertions(+), 236 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 6e6d9d39..b0f1f042 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -8643,39 +8643,6 @@ } } }, - "settings.app.renameSelectsName": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Rename Selects Existing Name" - } - } - } - }, - "settings.app.renameSelectsName.subtitleOff": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Command Palette rename keeps the caret at the end." - } - } - } - }, - "settings.app.renameSelectsName.subtitleOn": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Command Palette rename starts with all text selected." - } - } - } - }, "settings.app.reorderOnNotification": { "extractionState": "manual", "localizations": { diff --git a/Sources/CommandPaletteController.swift b/Sources/CommandPaletteController.swift index c12ebba8..f03aaf7a 100644 --- a/Sources/CommandPaletteController.swift +++ b/Sources/CommandPaletteController.swift @@ -65,8 +65,6 @@ final class CommandPaletteController: ObservableObject { @Published var commandPalettePendingActivation: ContentView.CommandPalettePendingActivation? @Published var commandPaletteResultsRevision: UInt64 = 0 @Published var commandPaletteUsageHistoryByCommandId: [String: ContentView.CommandPaletteUsageEntry] = [:] - @AppStorage(CommandPaletteRenameSelectionSettings.selectAllOnFocusKey) - var commandPaletteRenameSelectAllOnFocus = CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus @AppStorage(CommandPaletteSwitcherSearchSettings.searchAllSurfacesKey) var commandPaletteSearchAllSurfaces = CommandPaletteSwitcherSearchSettings.defaultSearchAllSurfaces @Published var commandPaletteShouldFocusWorkspaceDescriptionEditor = false diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index b330cf66..8d69c099 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -181,10 +181,6 @@ struct ContentView: View { get { commandPaletteController.commandPaletteUsageHistoryByCommandId } nonmutating set { commandPaletteController.commandPaletteUsageHistoryByCommandId = newValue } } - private var commandPaletteRenameSelectAllOnFocus: Bool { - get { commandPaletteController.commandPaletteRenameSelectAllOnFocus } - nonmutating set { commandPaletteController.commandPaletteRenameSelectAllOnFocus = newValue } - } private var commandPaletteSearchAllSurfaces: Bool { get { commandPaletteController.commandPaletteSearchAllSurfaces } nonmutating set { commandPaletteController.commandPaletteSearchAllSurfaces = newValue } @@ -5493,13 +5489,12 @@ struct ContentView: View { } private func commandPaletteRenameInputFocusPolicy() -> CommandPaletteInputFocusPolicy { - let selectAllOnFocus = CommandPaletteRenameSelectionSettings.selectAllOnFocusEnabled() - let selectionBehavior: CommandPaletteTextSelectionBehavior = selectAllOnFocus - ? .selectAll - : .caretAtEnd + // Rename always opens with the existing name selected, so typing replaces + // it. This used to be a preference. Other focus policies still use + // .caretAtEnd, so only this call site is fixed. return CommandPaletteInputFocusPolicy( focusTarget: .rename, - selectionBehavior: selectionBehavior + selectionBehavior: .selectAll ) } diff --git a/Sources/ProgramaSettingsFileStore.swift b/Sources/ProgramaSettingsFileStore.swift index 3ddc7f9e..3942d412 100644 --- a/Sources/ProgramaSettingsFileStore.swift +++ b/Sources/ProgramaSettingsFileStore.swift @@ -348,9 +348,6 @@ final class ProgramaSettingsFileStore { if let value = jsonBool(section["warnBeforeQuit"]) { snapshot.managedUserDefaults[QuitWarningSettings.warnBeforeQuitKey] = .bool(value) } - if let value = jsonBool(section["renameSelectsExistingName"]) { - snapshot.managedUserDefaults[CommandPaletteRenameSelectionSettings.selectAllOnFocusKey] = .bool(value) - } if let value = jsonBool(section["commandPaletteSearchesAllSurfaces"]) { snapshot.managedUserDefaults[CommandPaletteSwitcherSearchSettings.searchAllSurfacesKey] = .bool(value) } @@ -1207,7 +1204,6 @@ final class ProgramaSettingsFileStore { "preferredEditor": "", "reorderOnNotification": WorkspaceAutoReorderSettings.defaultValue, "warnBeforeQuit": QuitWarningSettings.defaultWarnBeforeQuit, - "renameSelectsExistingName": CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus, "commandPaletteSearchesAllSurfaces": CommandPaletteSwitcherSearchSettings.defaultSearchAllSurfaces, ], ], diff --git a/Sources/SettingsModels.swift b/Sources/SettingsModels.swift index b2e78cf6..a32ed9d4 100644 --- a/Sources/SettingsModels.swift +++ b/Sources/SettingsModels.swift @@ -105,16 +105,6 @@ enum ScrollbackPersistenceSettings { } } -enum CommandPaletteRenameSelectionSettings { - static let selectAllOnFocusKey = "commandPalette.renameSelectAllOnFocus" - static let defaultSelectAllOnFocus = true - private static let flag = UserDefaultsFlag(key: selectAllOnFocusKey, defaultValue: defaultSelectAllOnFocus) - - static func selectAllOnFocusEnabled(defaults: UserDefaults = .standard) -> Bool { - flag.isEnabled(defaults: defaults) - } -} - enum CommandPaletteSwitcherSearchSettings { static let searchAllSurfacesKey = "commandPalette.switcherSearchAllSurfaces" static let defaultSearchAllSurfaces = false diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index e4092fa5..4be0b8b1 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -53,8 +53,6 @@ struct SettingsView: View { private var longCommandThresholdSeconds = LongCommandNotificationSettings.defaultThresholdSeconds @AppStorage(QuitWarningSettings.warnBeforeQuitKey) private var warnBeforeQuitShortcut = QuitWarningSettings.defaultWarnBeforeQuit @AppStorage(ScrollbackPersistenceSettings.persistScrollbackKey) private var sessionPersistScrollback = ScrollbackPersistenceSettings.defaultPersistScrollback - @AppStorage(CommandPaletteRenameSelectionSettings.selectAllOnFocusKey) - private var commandPaletteRenameSelectAllOnFocus = CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus @AppStorage(CommandPaletteSwitcherSearchSettings.searchAllSurfacesKey) private var commandPaletteSearchAllSurfaces = CommandPaletteSwitcherSearchSettings.defaultSearchAllSurfaces @AppStorage(ShortcutHintDebugSettings.alwaysShowHintsKey) @@ -909,19 +907,6 @@ struct SettingsView: View { SettingsCardDivider() - SettingsCardRow( - String(localized: "settings.app.renameSelectsName", defaultValue: "Rename Selects Existing Name"), - subtitle: commandPaletteRenameSelectAllOnFocus - ? String(localized: "settings.app.renameSelectsName.subtitleOn", defaultValue: "Command Palette rename starts with all text selected.") - : String(localized: "settings.app.renameSelectsName.subtitleOff", defaultValue: "Command Palette rename keeps the caret at the end.") - ) { - Toggle("", isOn: $commandPaletteRenameSelectAllOnFocus) - .labelsHidden() - .controlSize(.small) - } - - SettingsCardDivider() - SettingsCardRow( String(localized: "settings.app.commandPaletteSearchAllSurfaces", defaultValue: "Command Palette Searches All Surfaces"), subtitle: commandPaletteSearchAllSurfaces @@ -1781,7 +1766,6 @@ struct SettingsView: View { showMenuBarExtra = MenuBarExtraSettings.defaultShowInMenuBar warnBeforeQuitShortcut = QuitWarningSettings.defaultWarnBeforeQuit sessionPersistScrollback = ScrollbackPersistenceSettings.defaultPersistScrollback - commandPaletteRenameSelectAllOnFocus = CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus commandPaletteSearchAllSurfaces = CommandPaletteSwitcherSearchSettings.defaultSearchAllSurfaces ShortcutHintDebugSettings.resetVisibilityDefaults() alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints diff --git a/Sources/TerminalController+Debug.swift b/Sources/TerminalController+Debug.swift index 4a455b57..ded5dbec 100644 --- a/Sources/TerminalController+Debug.swift +++ b/Sources/TerminalController+Debug.swift @@ -345,33 +345,6 @@ extension TerminalController { return result } - func v2DebugCommandPaletteRenameInputSelectAll(params: [String: Any]) -> V2CallResult { - if let rawEnabled = params["enabled"] { - guard let enabled = rawEnabled as? Bool else { - return .err( - code: "invalid_params", - message: "enabled must be a bool", - data: ["enabled": rawEnabled] - ) - } - DispatchQueue.main.sync { - UserDefaults.standard.set( - enabled, - forKey: CommandPaletteRenameSelectionSettings.selectAllOnFocusKey - ) - } - } - - var enabled = CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus - DispatchQueue.main.sync { - enabled = CommandPaletteRenameSelectionSettings.selectAllOnFocusEnabled() - } - - return .ok([ - "enabled": enabled - ]) - } - func v2DebugBrowserAddressBarFocused(params: [String: Any]) -> V2CallResult { let requestedSurfaceId = v2UUID(params, "surface_id") ?? v2UUID(params, "panel_id") var focusedSurfaceId: UUID? diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 18043b07..f05a918a 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -1988,8 +1988,6 @@ class TerminalController { return v2Result(id: id, self.v2DebugCommandPaletteRenameInputDeleteBackward(params: params)) case "debug.command_palette.rename_input.selection": return v2Result(id: id, self.v2DebugCommandPaletteRenameInputSelection(params: params)) - case "debug.command_palette.rename_input.select_all": - return v2Result(id: id, self.v2DebugCommandPaletteRenameInputSelectAll(params: params)) case "debug.browser.address_bar_focused": return v2Result(id: id, self.v2DebugBrowserAddressBarFocused(params: params)) case "debug.browser.favicon": diff --git a/Sources/V2CommandCatalog.swift b/Sources/V2CommandCatalog.swift index cc4bc29b..a17ab0e1 100644 --- a/Sources/V2CommandCatalog.swift +++ b/Sources/V2CommandCatalog.swift @@ -228,7 +228,6 @@ enum V2CommandCatalog { "debug.command_palette.rename_input.interact", "debug.command_palette.rename_input.delete_backward", "debug.command_palette.rename_input.selection", - "debug.command_palette.rename_input.select_all", "debug.browser.address_bar_focused", "debug.browser.favicon", "debug.sidebar.visible", diff --git a/programaTests/ShortcutAndCommandPaletteTests.swift b/programaTests/ShortcutAndCommandPaletteTests.swift index ee8d4cbd..a34fe4e9 100644 --- a/programaTests/ShortcutAndCommandPaletteTests.swift +++ b/programaTests/ShortcutAndCommandPaletteTests.swift @@ -680,34 +680,6 @@ final class CommandPaletteRestoreFocusStateMachineTests: XCTestCase { } -final class CommandPaletteRenameSelectionSettingsTests: XCTestCase { - private let suiteName = "cmux.tests.commandPaletteRenameSelection.\(UUID().uuidString)" - - private func makeDefaults() -> UserDefaults { - let defaults = UserDefaults(suiteName: suiteName)! - defaults.removePersistentDomain(forName: suiteName) - return defaults - } - - func testDefaultsToSelectAllWhenUnset() { - let defaults = makeDefaults() - XCTAssertTrue(CommandPaletteRenameSelectionSettings.selectAllOnFocusEnabled(defaults: defaults)) - } - - func testReturnsFalseWhenStoredFalse() { - let defaults = makeDefaults() - defaults.set(false, forKey: CommandPaletteRenameSelectionSettings.selectAllOnFocusKey) - XCTAssertFalse(CommandPaletteRenameSelectionSettings.selectAllOnFocusEnabled(defaults: defaults)) - } - - func testReturnsTrueWhenStoredTrue() { - let defaults = makeDefaults() - defaults.set(true, forKey: CommandPaletteRenameSelectionSettings.selectAllOnFocusKey) - XCTAssertTrue(CommandPaletteRenameSelectionSettings.selectAllOnFocusEnabled(defaults: defaults)) - } -} - - final class CommandPaletteSelectionScrollBehaviorTests: XCTestCase { func testFirstEntryPinsToTopAnchor() { let anchor = ContentView.commandPaletteScrollPositionAnchor( diff --git a/tests_v2/cmux.py b/tests_v2/cmux.py index 6a411617..4edc1862 100755 --- a/tests_v2/cmux.py +++ b/tests_v2/cmux.py @@ -951,14 +951,6 @@ def command_palette_results(self, window_id: str, limit: int = 20) -> dict: ) or {} return dict(res) - def command_palette_rename_select_all(self) -> bool: - res = self._call("debug.command_palette.rename_input.select_all") or {} - return bool(res.get("enabled")) - - def set_command_palette_rename_select_all(self, enabled: bool) -> bool: - res = self._call("debug.command_palette.rename_input.select_all", {"enabled": bool(enabled)}) or {} - return bool(res.get("enabled")) - def is_terminal_focused(self, panel: Union[str, int]) -> bool: sid = self._resolve_surface_id(panel) res = self._call("debug.terminal.is_focused", {"surface_id": sid}) or {} diff --git a/tests_v2/test_command_palette_backspace_go_back.py b/tests_v2/test_command_palette_backspace_go_back.py index 3504c9fe..e48390dd 100644 --- a/tests_v2/test_command_palette_backspace_go_back.py +++ b/tests_v2/test_command_palette_backspace_go_back.py @@ -77,10 +77,9 @@ def main(): time.sleep(0.2) window_id = client.current_window() - original_select_all = client.command_palette_rename_select_all() - try: - client.set_command_palette_rename_select_all(True) + # Rename always opens with the existing name selected now, so this + # no longer has to pin a setting to get a deterministic starting state. _open_rename_input(client, window_id) _wait_until( @@ -138,11 +137,6 @@ def main(): raise cmuxError("palette closed unexpectedly instead of navigating back to command list") finally: - try: - client.set_command_palette_rename_select_all(original_select_all) - except Exception: - pass - if _palette_visible(client, window_id): client._call("debug.command_palette.toggle", {"window_id": window_id}) _wait_until( diff --git a/tests_v2/test_command_palette_rename_select_all.py b/tests_v2/test_command_palette_rename_select_all.py index 63e9c07d..252c990a 100644 --- a/tests_v2/test_command_palette_rename_select_all.py +++ b/tests_v2/test_command_palette_rename_select_all.py @@ -3,8 +3,12 @@ Regression test: command-palette rename input keeps select-all on interaction. Coverage: -- With select-all setting enabled, rename input selects all existing text - immediately and stays selected after interaction. +- Rename input selects all existing text immediately on open, and the + selection survives interaction with the field. + +Rename selecting the existing name used to be a setting. It is now the only +behaviour, so this exercises it directly instead of toggling +`debug.command_palette.rename_input.select_all`, which no longer exists. """ import os @@ -37,26 +41,7 @@ def _rename_input_selection(client, window_id): return client._call("debug.command_palette.rename_input.selection", {"window_id": window_id}) or {} -def _rename_select_all_setting(client): - payload = client._call("debug.command_palette.rename_input.select_all", {}) or {} - return bool(payload.get("enabled")) - - -def _set_rename_select_all_setting(client, enabled): - payload = client._call( - "debug.command_palette.rename_input.select_all", - {"enabled": bool(enabled)}, - ) or {} - return bool(payload.get("enabled")) - - -def _wait_for_rename_selection( - client, - window_id, - expect_select_all, - message, - timeout_s=0.6, -): +def _wait_for_select_all(client, window_id, message, timeout_s=0.6): def _matches(): selection = _rename_input_selection(client, window_id) if not selection.get("focused"): @@ -64,40 +49,49 @@ def _matches(): text_length = int(selection.get("text_length") or 0) selection_location = int(selection.get("selection_location") or 0) selection_length = int(selection.get("selection_length") or 0) - if expect_select_all: - return text_length > 0 and selection_location == 0 and selection_length == text_length - return selection_location == text_length and selection_length == 0 + return text_length > 0 and selection_location == 0 and selection_length == text_length _wait_until(_matches, timeout_s=timeout_s, message=message) -def _exercise_rename_selection_setting( - client, - window_id, - expect_select_all, - cycles, - label, -): +def _open_rename_tab_input(client, window_id): + client.activate_app() + client.focus_window(window_id) + time.sleep(0.1) + + if _palette_visible(client, window_id): + client._call("debug.command_palette.toggle", {"window_id": window_id}) + _wait_until( + lambda: not _palette_visible(client, window_id), + message="command palette failed to close before setup", + ) + + client.open_command_palette_rename_tab_input(window_id=window_id) + _wait_until( + lambda: _palette_visible(client, window_id), + message="command palette failed to open rename-tab input", + ) + + +def _exercise_rename_selection(client, window_id, cycles): for cycle in range(cycles): _open_rename_tab_input(client, window_id) - _wait_for_rename_selection( + _wait_for_select_all( client, window_id, - expect_select_all=expect_select_all, timeout_s=0.4, message=( - f"{label}: rename input not ready with expected selection " - f"on open (cycle {cycle + 1}/{cycles})" + "rename input not ready with the name selected on open " + f"(cycle {cycle + 1}/{cycles})" ), ) client._call("debug.command_palette.rename_input.interact", {"window_id": window_id}) - _wait_for_rename_selection( + _wait_for_select_all( client, window_id, - expect_select_all=expect_select_all, timeout_s=0.6, message=( - f"{label}: rename input selection changed after interaction " + "rename input selection changed after interaction " f"(cycle {cycle + 1}/{cycles})" ), ) @@ -106,36 +100,15 @@ def _exercise_rename_selection_setting( client._call("debug.command_palette.toggle", {"window_id": window_id}) _wait_until( lambda: not _palette_visible(client, window_id), - message=f"{label}: command palette failed to close (cycle {cycle + 1}/{cycles})", + message=f"command palette failed to close (cycle {cycle + 1}/{cycles})", ) -def _open_rename_tab_input(client, window_id): - client.activate_app() - client.focus_window(window_id) - time.sleep(0.1) - - if _palette_visible(client, window_id): - client._call("debug.command_palette.toggle", {"window_id": window_id}) - _wait_until( - lambda: not _palette_visible(client, window_id), - message="command palette failed to close before setup", - ) - - client.open_command_palette_rename_tab_input(window_id=window_id) - _wait_until( - lambda: _palette_visible(client, window_id), - message="command palette failed to open rename-tab input", - ) - - def main(): with cmux(SOCKET_PATH) as client: client.activate_app() time.sleep(0.2) - original_select_all = _rename_select_all_setting(client) - workspace_id = client.new_workspace() client.select_workspace(workspace_id) client.rename_workspace("SeedName", workspace_id) @@ -143,33 +116,8 @@ def main(): window_id = client.current_window() try: - stress_cycles = 8 - - # ON: immediate select-all and interaction-preserved select-all. - _set_rename_select_all_setting(client, True) - _exercise_rename_selection_setting( - client, - window_id, - expect_select_all=True, - cycles=stress_cycles, - label="select-all enabled", - ) - - # OFF: immediate caret-at-end and interaction-preserved caret-at-end. - _set_rename_select_all_setting(client, False) - _exercise_rename_selection_setting( - client, - window_id, - expect_select_all=False, - cycles=stress_cycles, - label="select-all disabled", - ) - + _exercise_rename_selection(client, window_id, cycles=8) finally: - try: - _set_rename_select_all_setting(client, original_select_all) - except Exception: - pass if _palette_visible(client, window_id): client._call("debug.command_palette.toggle", {"window_id": window_id}) _wait_until( @@ -177,7 +125,7 @@ def main(): message="command palette failed to close during cleanup", ) - print("PASS: command-palette rename input obeys select-all setting (on/off)") + print("PASS: command-palette rename input selects the existing name and keeps it") return 0 From 36e37ed557b69ce5071dd7acffdba0705928598c Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 12:47:16 -0300 Subject: [PATCH 08/11] test: file the socket-driven perf harnesses with the other socket tests tests/ and tests_v2/ both held python, which made the split look like versioning. It is not: tests_v2 is named for the v2 socket API, and every file there drives a running app over the control socket. Two files were on the wrong side of that line. test_text_input_refresh_cost.py and test_workspace_churn_up_arrow_lag.py both connect to a live instance, so they move to tests_v2 and their ci.yml invocations move with them. The three python files left in tests/ drive the programa CLI binary as a subprocess and need no app, which is what tests/ is for alongside the shell script guards. Adds docs/testing-layout.md explaining the four harnesses, what _v2 actually refers to, and which directory a new test belongs in. That question came up because nothing in the repo answered it. --- .github/workflows/ci.yml | 4 +- docs/testing-layout.md | 48 +++++++++++++++++++ .../test_text_input_refresh_cost.py | 8 ++-- .../test_workspace_churn_up_arrow_lag.py | 0 4 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 docs/testing-layout.md rename {tests => tests_v2}/test_text_input_refresh_cost.py (96%) rename {tests => tests_v2}/test_workspace_churn_up_arrow_lag.py (100%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 014c6f77..7866ccaf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -689,7 +689,7 @@ jobs: PROGRAMA_LAG_MAX_CHURN_P95_MS=35 \ PROGRAMA_LAG_KEY_EVENTS=180 \ PROGRAMA_LAG_KEY_COMBO="${{ inputs.lag_combo || 'up' }}" \ - python3 tests/test_workspace_churn_up_arrow_lag.py + python3 tests_v2/test_workspace_churn_up_arrow_lag.py # On-demand only. This measures the refresh call itself with an in-process # timer rather than an end-to-end socket round trip, because that round trip @@ -739,7 +739,7 @@ jobs: # where nobody can read it. That cost a full four-run cycle to learn nothing. if ! PROGRAMA_SOCKET_PATH="$SOCK" \ PROGRAMA_REFRESH_COST_KEY_EVENTS=400 \ - python3 tests/test_text_input_refresh_cost.py | tee -a "$GITHUB_STEP_SUMMARY"; then + python3 tests_v2/test_text_input_refresh_cost.py | tee -a "$GITHUB_STEP_SUMMARY"; then echo "--- app still running? ---" pgrep -x "Programa DEV" >/dev/null && echo "yes" || echo "NO - it exited" echo "--- socket present at $SOCK? ---" diff --git a/docs/testing-layout.md b/docs/testing-layout.md new file mode 100644 index 00000000..9d8265a3 --- /dev/null +++ b/docs/testing-layout.md @@ -0,0 +1,48 @@ +# Where tests live + +Four directories, because there are four harnesses. They are not versions of +each other, and `tests_v2` is not "the second attempt at `tests`". + +| Directory | Language | Needs a running app? | Run by | +|---|---|---|---| +| `tests/` | shell + python | no | `workflow-guard-tests`, CLI steps in `ci.yml` | +| `tests_v2/` | python | **yes** | `socket-integration-tests`, lag/perf jobs | +| `programaTests/` | Swift (XCTest) | no | `unit-tests` (`programa-unit` scheme) | +| `programaUITests/` | Swift (XCUITest) | launches its own | `ui-regressions` | + +## What `_v2` means + +The v2 **socket API** — the JSON-RPC surface in `Sources/V2CommandCatalog.swift`, +whose methods look like `debug.command_palette.visible` or `snapshot.restore`. +Those tests connect to a running Programa over its control socket and drive the +app through that API. The name is about the protocol, not about a previous +generation of tests. There is no `tests_v1`. + +## Which one to write in + +**`tests/`** — the thing under test is a script or a build artifact, and no app +is involved. Shell scripts here guard `scripts/*.sh`, the CI workflows, DMG +creation, and release assets. The three python files here drive the `programa` +CLI binary as a subprocess via `PROGRAMA_CLI_BIN`. + +**`tests_v2/`** — the thing under test is app behaviour you can observe over the +socket. Everything here talks to a live instance, whether through `cmux.py` or +by speaking JSON-RPC directly. The two lag/perf harnesses live here for that +reason, even though they bypass `cmux.py`. + +**`programaTests/`** — pure logic that can be exercised without launching +anything: policy types, decision functions, snapshot encoding, layout maths. +Cheapest and fastest, so prefer it when a behaviour can be reached this way. +When it cannot, add a small seam so it can, rather than reaching for a socket +test. See the test-quality policy in `CLAUDE.md`. + +**`programaUITests/`** — behaviour that only exists through real event +delivery and AppKit: clicks, drags, menus, focus. + +## Running them + +Per `CLAUDE.md`, tests are **not run locally** — they run on CI or the VM. +`xcodebuild -scheme programa-unit` is the one safe local exception because it +launches no app. `tests_v2` in particular will attach to whatever socket it +finds, which is why running it locally risks driving your real Programa +instance rather than a build under test. diff --git a/tests/test_text_input_refresh_cost.py b/tests_v2/test_text_input_refresh_cost.py similarity index 96% rename from tests/test_text_input_refresh_cost.py rename to tests_v2/test_text_input_refresh_cost.py index 84403017..9bd26d14 100644 --- a/tests/test_text_input_refresh_cost.py +++ b/tests_v2/test_text_input_refresh_cost.py @@ -3,7 +3,7 @@ Measuring instrument (not a regression gate): in-process cost of the text-input refresh call on the keyDown path. -tests/test_workspace_churn_up_arrow_lag.py measures a full socket RPC round trip +tests_v2/test_workspace_churn_up_arrow_lag.py measures a full socket RPC round trip (IPC + JSON + DispatchQueue.main.sync + AppKit dispatch + the work under test). That transport floor sits in the low milliseconds, which is too coarse to resolve changes to a single call inside keyDown (e.g. swapping forceRefresh() for a @@ -41,7 +41,7 @@ import time from typing import Optional -# Mirrors tests/test_workspace_churn_up_arrow_lag.py: speak the v2 JSON-RPC +# Mirrors tests_v2/test_workspace_churn_up_arrow_lag.py: speak the v2 JSON-RPC # protocol directly for the tight simulate-keystroke loop, and reuse # tests_v2/cmux.py only for its error type. _TESTS_V2_DIR = os.path.normpath( @@ -65,7 +65,7 @@ class RawSocketClient: """Minimal v2 JSON-RPC client for the simulate-keystroke loop. Copied in shape from RawSocketClient in - tests/test_workspace_churn_up_arrow_lag.py: skips the full tests_v2/cmux.py + tests_v2/test_workspace_churn_up_arrow_lag.py: skips the full tests_v2/cmux.py client's id-resolution helpers so per-call overhead stays minimal. Not shared with that file directly since it is out of scope for this change. """ @@ -142,7 +142,7 @@ def call(self, method: str, params: Optional[dict] = None, timeout_s: float = 2. def resolve_target_socket() -> str: - # Same refusal as tests/test_workspace_churn_up_arrow_lag.py::resolve_target_socket: + # Same refusal as tests_v2/test_workspace_churn_up_arrow_lag.py::resolve_target_socket: # never target the main/untagged socket from an automated harness. socket_path = os.environ.get("PROGRAMA_SOCKET_PATH") if not socket_path: diff --git a/tests/test_workspace_churn_up_arrow_lag.py b/tests_v2/test_workspace_churn_up_arrow_lag.py similarity index 100% rename from tests/test_workspace_churn_up_arrow_lag.py rename to tests_v2/test_workspace_churn_up_arrow_lag.py From f6ecd553bfcfbe8caa6cea3cca9c6a3c02327c85 Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 13:04:33 -0300 Subject: [PATCH 09/11] test: drop the tests for the removed keep-workspace-open preference CI caught three tests that still exercised the preference deleted earlier in this branch. They compiled because they write the raw defaults key rather than referencing the enum, so only running them surfaced it: - TabManagerCloseCurrentPanelTests.testCloseCurrentPanelKeepsWorkspaceOpen... failed outright. It set the key and expected the workspace to survive. - testClosePanelButtonStillClosesWorkspaceWhenKeepWorkspaceOpen... still passed, but only by duplicating the plain close-button test now that the preference does nothing. - AppDelegateShortcutRoutingTests.testCmdWKeepsLastSurfaceWorkspaceOpen... had the same premise. Replaced with one guard modelled on the two-workspace close test that already passes, writing the removed key first and asserting it changes nothing. That covers what actually matters: a stale settings.json entry must not bring the old behaviour back. The earlier cut verified against Sources/ and CLI/ but not programaTests/, which is how these survived. --- .../AppDelegateShortcutRoutingTests.swift | 63 ----------- programaTests/TabManagerUnitTests.swift | 107 ++++++------------ 2 files changed, 36 insertions(+), 134 deletions(-) diff --git a/programaTests/AppDelegateShortcutRoutingTests.swift b/programaTests/AppDelegateShortcutRoutingTests.swift index 7dee2f43..c05571f7 100644 --- a/programaTests/AppDelegateShortcutRoutingTests.swift +++ b/programaTests/AppDelegateShortcutRoutingTests.swift @@ -1468,69 +1468,6 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { ) } - func testCmdWKeepsLastSurfaceWorkspaceOpenWhenKeepWorkspaceOpenPreferenceIsEnabled() throws { - guard let appDelegate = AppDelegate.shared else { - XCTFail("Expected AppDelegate.shared") - return - } - - appDelegate.debugCloseMainWindowConfirmationHandler = { _ in true } - - let defaults = UserDefaults.standard - let originalSetting = defaults.object(forKey: appDelegateLastSurfaceCloseShortcutDefaultsKey) - defaults.set(false, forKey: appDelegateLastSurfaceCloseShortcutDefaultsKey) - defer { - if let originalSetting { - defaults.set(originalSetting, forKey: appDelegateLastSurfaceCloseShortcutDefaultsKey) - } else { - defaults.removeObject(forKey: appDelegateLastSurfaceCloseShortcutDefaultsKey) - } - } - - let windowId = appDelegate.createMainWindow() - defer { closeWindow(withId: windowId) } - - guard let targetWindow = window(withId: windowId), - let manager = appDelegate.tabManagerFor(windowId: windowId), - let workspace = manager.selectedWorkspace, - let initialPanelId = workspace.focusedPanelId else { - XCTFail("Expected test window, manager, workspace, and focused panel") - return - } - - // This test exercises keep-workspace-open semantics, not close-confirm heuristics. - // Mark the shell idle so Cmd+W routes through the immediate close path deterministically. - workspace.updatePanelShellActivityState(panelId: initialPanelId, state: .promptIdle) - - guard let event = makeKeyDownEvent( - key: "w", - modifiers: [.command], - keyCode: 13, - windowNumber: targetWindow.windowNumber - ) else { - XCTFail("Failed to construct Cmd+W event") - return - } - -#if DEBUG - XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) -#else - XCTFail("debugHandleCustomShortcut is only available in DEBUG") -#endif - - waitUntil(description: "the closed panel to be removed from the workspace") { workspace.panels[initialPanelId] == nil } - - XCTAssertNotNil( - self.window(withId: windowId), - "Cmd+W should keep the window open when the keep-workspace-open preference is enabled" - ) - XCTAssertEqual(manager.tabs.count, 1) - XCTAssertEqual(manager.selectedTabId, workspace.id) - XCTAssertNil(workspace.panels[initialPanelId]) - XCTAssertEqual(workspace.panels.count, 1) - XCTAssertNotEqual(workspace.focusedPanelId, initialPanelId) - } - func testCmdWClosesAuxiliaryWindowInsteadOfMainTerminalPanel() throws { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") diff --git a/programaTests/TabManagerUnitTests.swift b/programaTests/TabManagerUnitTests.swift index c3909ff5..48677218 100644 --- a/programaTests/TabManagerUnitTests.swift +++ b/programaTests/TabManagerUnitTests.swift @@ -1050,6 +1050,42 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase { XCTAssertTrue(secondWorkspace.panels.isEmpty) } + // Mirrors testCloseCurrentPanelClosesWorkspaceWhenItOwnsTheLastSurface, with the + // removed preference key written first. Cmd+W closing the workspace along with + // its last surface is no longer configurable, and a stale settings.json entry + // must not quietly bring the old behaviour back. + func testCloseCurrentPanelClosesWorkspaceEvenWhenRemovedKeepOpenKeyIsSet() { + let defaults = UserDefaults.standard + let original = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey) + defaults.set(false, forKey: lastSurfaceCloseShortcutDefaultsKey) + defer { + if let original { + defaults.set(original, forKey: lastSurfaceCloseShortcutDefaultsKey) + } else { + defaults.removeObject(forKey: lastSurfaceCloseShortcutDefaultsKey) + } + } + + let manager = TabManager() + let firstWorkspace = manager.tabs[0] + let secondWorkspace = manager.addWorkspace() + manager.selectWorkspace(secondWorkspace) + + guard let secondPanelId = secondWorkspace.focusedPanelId else { + XCTFail("Expected focused panel in selected workspace") + return + } + + manager.closeCurrentPanelWithConfirmation() + drainMainQueue() + drainMainQueue() + + XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id]) + XCTAssertEqual(manager.selectedTabId, firstWorkspace.id) + XCTAssertNil(secondWorkspace.panels[secondPanelId]) + XCTAssertTrue(secondWorkspace.panels.isEmpty) + } + func testCloseCurrentPanelPromptsBeforeClosingPinnedWorkspaceLastSurface() { let manager = TabManager() _ = manager.tabs[0] @@ -1119,39 +1155,6 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase { XCTAssertTrue(pinnedWorkspace.panels.isEmpty) } - func testCloseCurrentPanelKeepsWorkspaceOpenWhenKeepWorkspaceOpenPreferenceIsEnabled() { - let defaults = UserDefaults.standard - let originalSetting = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey) - defaults.set(false, forKey: lastSurfaceCloseShortcutDefaultsKey) - defer { - if let originalSetting { - defaults.set(originalSetting, forKey: lastSurfaceCloseShortcutDefaultsKey) - } else { - defaults.removeObject(forKey: lastSurfaceCloseShortcutDefaultsKey) - } - } - - let manager = TabManager() - guard let workspace = manager.selectedWorkspace, - let initialPanelId = workspace.focusedPanelId else { - XCTFail("Expected selected workspace and focused panel") - return - } - - let initialWorkspaceId = workspace.id - - manager.closeCurrentPanelWithConfirmation() - drainMainQueue() - drainMainQueue() - - XCTAssertEqual(manager.tabs.count, 1) - XCTAssertEqual(manager.selectedTabId, initialWorkspaceId) - XCTAssertEqual(manager.tabs.first?.id, initialWorkspaceId) - XCTAssertNil(workspace.panels[initialPanelId]) - XCTAssertEqual(workspace.panels.count, 1) - XCTAssertNotEqual(workspace.focusedPanelId, initialPanelId) - } - func testClosePanelButtonClosesWorkspaceWhenItOwnsTheLastSurface() { let manager = TabManager() let firstWorkspace = manager.tabs[0] @@ -1182,44 +1185,6 @@ final class TabManagerCloseCurrentPanelTests: XCTestCase { XCTAssertTrue(secondWorkspace.panels.isEmpty) } - func testClosePanelButtonStillClosesWorkspaceWhenKeepWorkspaceOpenPreferenceIsEnabled() { - let defaults = UserDefaults.standard - let originalSetting = defaults.object(forKey: lastSurfaceCloseShortcutDefaultsKey) - defaults.set(false, forKey: lastSurfaceCloseShortcutDefaultsKey) - defer { - if let originalSetting { - defaults.set(originalSetting, forKey: lastSurfaceCloseShortcutDefaultsKey) - } else { - defaults.removeObject(forKey: lastSurfaceCloseShortcutDefaultsKey) - } - } - - let manager = TabManager() - let firstWorkspace = manager.tabs[0] - let secondWorkspace = manager.addWorkspace() - manager.selectWorkspace(secondWorkspace) - - guard let secondPanelId = secondWorkspace.focusedPanelId else { - XCTFail("Expected focused panel in selected workspace") - return - } - - guard let secondSurfaceId = secondWorkspace.surfaceIdFromPanelId(secondPanelId) else { - XCTFail("Expected bonsplit surface ID for focused panel") - return - } - - secondWorkspace.markExplicitClose(surfaceId: secondSurfaceId) - XCTAssertFalse(secondWorkspace.closePanel(secondPanelId)) - drainMainQueue() - drainMainQueue() - - XCTAssertEqual(manager.tabs.map(\.id), [firstWorkspace.id]) - XCTAssertEqual(manager.selectedTabId, firstWorkspace.id) - XCTAssertNil(secondWorkspace.panels[secondPanelId]) - XCTAssertTrue(secondWorkspace.panels.isEmpty) - } - func testGenericClosePanelKeepsWorkspaceOpenWithoutExplicitCloseMarker() { let manager = TabManager() guard let workspace = manager.selectedWorkspace, From 859dc3a5c692d27f449c04d02b4de2172ffc4b63 Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 13:01:58 -0300 Subject: [PATCH 10/11] refactor(settings): stop duplicating the import hint inside Settings The Browser section rendered a mock of the blank-tab import hint, reusing the hint's own strings. That included its footnote, "You can always find this in Settings > Browser", displayed to someone already looking at Settings. The Choose and Refresh buttons directly below it already offer the action the mock was advertising, so the mock only added noise. The real card is unchanged in BrowserToolbarViews, and both strings are still used there. browserImportSubtitle went with it, since the mock was its only caller. --- Sources/SettingsView.swift | 41 +++++++------------------------------- 1 file changed, 7 insertions(+), 34 deletions(-) diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index 4be0b8b1..1c4b722f 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -213,10 +213,6 @@ struct SettingsView: View { } } - private var browserImportSubtitle: String { - InstalledBrowserDetector.summaryText(for: detectedImportBrowsers) - } - private var browserImportHintSettingsNote: String { switch browserImportHintPresentation.settingsStatus { case .visible: @@ -1580,36 +1576,13 @@ struct SettingsView: View { SettingsCardDivider() VStack(alignment: .leading, spacing: 12) { - VStack(alignment: .leading, spacing: 8) { - Text(String(localized: "settings.browser.import", defaultValue: "Import Browser Data")) - .font(.system(size: 13, weight: .semibold)) - - VStack(alignment: .leading, spacing: 6) { - Text(String(localized: "browser.import.hint.title", defaultValue: "Import browser data")) - .font(.system(size: 12.5, weight: .semibold)) - - Text(browserImportSubtitle) - .font(.caption) - .foregroundStyle(.secondary) - .fixedSize(horizontal: false, vertical: true) - - Text(String(localized: "browser.import.hint.settingsFootnote", defaultValue: "You can always find this in Settings > Browser.")) - .font(.system(size: 10.5)) - .foregroundStyle(.tertiary) - .fixedSize(horizontal: false, vertical: true) - } - .padding(.horizontal, 12) - .padding(.vertical, 10) - .frame(maxWidth: .infinity, alignment: .leading) - .background( - RoundedRectangle(cornerRadius: 10, style: .continuous) - .fill(Color(nsColor: .controlBackgroundColor)) - ) - .overlay( - RoundedRectangle(cornerRadius: 10, style: .continuous) - .stroke(Color(nsColor: .separatorColor).opacity(0.4), lineWidth: 1) - ) - } + // A mock of the blank-tab import hint used to be rendered here, + // reusing the hint's own strings -- including its footnote saying + // "You can always find this in Settings > Browser", shown inside + // Settings. The buttons below do the same job without restating + // the hint. The real card still lives in BrowserToolbarViews. + Text(String(localized: "settings.browser.import", defaultValue: "Import Browser Data")) + .font(.system(size: 13, weight: .semibold)) HStack(spacing: 8) { Button(String(localized: "settings.browser.import.choose", defaultValue: "Choose…")) { From 47c529f19ce33ee95ebee7188410c71e74ce4c74 Mon Sep 17 00:00:00 2001 From: arzafran Date: Fri, 31 Jul 2026 13:32:29 -0300 Subject: [PATCH 11/11] refactor(pairing): one way in on each side instead of three Pairing offered three entry points and two buttons for a single action, and the two devices did not agree on what pairing even looked like. The phone had: scan a QR, paste a combined code with a "Use This Code" button, and an Advanced section with separate ticket and token fields behind its own "Connect" button. Two buttons that both connect, one of which only worked with the fields nobody was meant to use. Its own doc comment called the split fields legacy. The Mac had: a QR, the combined code shown as selectable text with a Copy button, and below that a "Can't scan? Paste the payload and token manually" disclosure repeating the same secret split in two. Both legacy paths are gone. The Mac shows the QR and the one combined code that the QR encodes; copying that is the fallback for anyone who cannot scan. The phone scans or pastes that code, and its single button is now called Connect because connecting is what it does. The ticket and token drafts stay on AppStore, since applyPairingCode still fills them and connectManually still reads them. They are just no longer bound to fields. The Connect button also picks up the isConnecting guard the removed button used to own. Both apps build. Nine localization keys dropped across the two catalogs, and pairing.connect.useCodeButton retranslated for its new label. --- Resources/Localizable.xcstrings | 57 +----------- Sources/SettingsView.swift | 30 +------ .../ProgramaSpike/PairConnectView.swift | 61 +++---------- .../Shared/Localizable.xcstrings | 89 +------------------ 4 files changed, 18 insertions(+), 219 deletions(-) diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index b0f1f042..bfd42ffd 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -10218,23 +10218,6 @@ } } }, - "settings.phone.pair.manualFallback": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Can't scan? Paste the payload and token manually" - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "スキャンできない場合は、ペイロードとトークンを手動で貼り付け" - } - } - } - }, "settings.phone.pair.scanLabel": { "extractionState": "manual", "localizations": { @@ -10252,36 +10235,19 @@ } } }, - "settings.phone.pair.subtitle": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Opens a single-use, 5-minute pairing window. Scan the QR code with the Programa iOS app, or enter the payload and token manually." - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "5分間有効な、1回限りのペア設定ウィンドウを開きます。Programa iOSアプリでQRコードをスキャンするか、ペイロードとトークンを手動で入力してください。" - } - } - } - }, - "settings.phone.pair.ticketLabel": { + "settings.phone.pair.subtitleV2": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Pairing Payload" + "value": "Opens a single-use, 5-minute pairing window. Scan the QR code with the Programa iOS app, or copy the code it shows." } }, "ja": { "stringUnit": { "state": "translated", - "value": "ペア設定ペイロード" + "value": "一度だけ使える5分間のペアリング枠を開きます。Programa iOSアプリでQRコードをスキャンするか、表示されたコードをコピーしてください。" } } } @@ -10303,23 +10269,6 @@ } } }, - "settings.phone.pair.tokenLabel": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Pairing Token" - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "ペア設定トークン" - } - } - } - }, "settings.preset.hudGlass": { "extractionState": "manual", "localizations": { diff --git a/Sources/SettingsView.swift b/Sources/SettingsView.swift index 1c4b722f..82a74f62 100644 --- a/Sources/SettingsView.swift +++ b/Sources/SettingsView.swift @@ -1229,7 +1229,7 @@ struct SettingsView: View { SettingsCardRow( String(localized: "settings.phone.pair.title", defaultValue: "Pair a Device"), - subtitle: String(localized: "settings.phone.pair.subtitle", defaultValue: "Opens a single-use, 5-minute pairing window. Scan the QR code with the Programa iOS app, or enter the payload and token manually.") + subtitle: String(localized: "settings.phone.pair.subtitleV2", defaultValue: "Opens a single-use, 5-minute pairing window. Scan the QR code with the Programa iOS app, or copy the code it shows.") ) { Button(String(localized: "settings.phone.pair.button", defaultValue: "Pair a Device…")) { beginMobileBridgePairing() @@ -1287,34 +1287,6 @@ struct SettingsView: View { } } - DisclosureGroup(String(localized: "settings.phone.pair.manualFallback", defaultValue: "Can't scan? Paste the payload and token manually")) { - VStack(alignment: .leading, spacing: 8) { - Text(String(localized: "settings.phone.pair.ticketLabel", defaultValue: "Pairing Payload")) - .font(.system(size: 12, weight: .semibold)) - Text(ticket) - .font(.system(size: 11, design: .monospaced)) - .textSelection(.enabled) - .padding(8) - .frame(maxWidth: .infinity, alignment: .leading) - .background( - RoundedRectangle(cornerRadius: 6, style: .continuous) - .fill(Color(nsColor: .controlBackgroundColor)) - ) - - Text(String(localized: "settings.phone.pair.tokenLabel", defaultValue: "Pairing Token")) - .font(.system(size: 12, weight: .semibold)) - Text(token) - .font(.system(size: 11, design: .monospaced)) - .textSelection(.enabled) - .padding(8) - .frame(maxWidth: .infinity, alignment: .leading) - .background( - RoundedRectangle(cornerRadius: 6, style: .continuous) - .fill(Color(nsColor: .controlBackgroundColor)) - ) - } - .padding(.top, 6) - } } .padding(.horizontal, 14) .padding(.vertical, 10) diff --git a/ios/ProgramaSpike/ProgramaSpike/PairConnectView.swift b/ios/ProgramaSpike/ProgramaSpike/PairConnectView.swift index a48abaed..67503944 100644 --- a/ios/ProgramaSpike/ProgramaSpike/PairConnectView.swift +++ b/ios/ProgramaSpike/ProgramaSpike/PairConnectView.swift @@ -1,9 +1,13 @@ import SwiftUI /// Screen 1: scan or paste the pairing code shown on Programa's Mac -/// Settings ▸ Phone screen, then connect. The legacy separate ticket/token -/// fields stay as a fallback for testers who can't scan or whose combined -/// code paste didn't parse. +/// Settings ▸ Phone screen, then connect. +/// +/// There is deliberately one way in. Separate ticket and token fields used to +/// sit below this as a legacy fallback, with their own Connect button, so the +/// screen offered three entry points and two buttons for a single action. The +/// Mac shows the same combined code it encodes in the QR, and pasting that is +/// the fallback for anyone who cannot scan. struct PairConnectView: View { @Bindable var store: AppStore @@ -36,10 +40,13 @@ struct PairConnectView: View { .textInputAutocapitalization(.never) .autocorrectionDisabled() - Button(String(localized: "pairing.connect.useCodeButton", defaultValue: "Use This Code")) { + Button(String(localized: "pairing.connect.useCodeButton", defaultValue: "Connect")) { applyPairingCodeDraft() } - .disabled(pairingCodeDraft.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) + .disabled( + pairingCodeDraft.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + || store.isConnecting + ) if let pairingCodeError { Text(pairingCodeError) @@ -48,50 +55,6 @@ struct PairConnectView: View { } } - Section( - String( - localized: "pairing.connect.section.advanced", - defaultValue: "Advanced: paste ticket and token separately" - ) - ) { - TextField( - String( - localized: "pairing.connect.ticketField.placeholder", - defaultValue: "Paste the ticket from Programa's pairing screen" - ), - text: $store.pairingTicketDraft, - axis: .vertical - ) - .lineLimit(3 ... 8) - .textInputAutocapitalization(.never) - .autocorrectionDisabled() - - TextField( - String(localized: "pairing.connect.tokenField.placeholder", defaultValue: "Only needed the first time"), - text: $store.pairingTokenDraft - ) - .textInputAutocapitalization(.never) - .autocorrectionDisabled() - Text( - String( - localized: "pairing.connect.tokenField.footnote", - defaultValue: "Once this device is paired it stays trusted — you won't need the token again." - ) - ) - .font(.footnote) - .foregroundStyle(.secondary) - } - - Section { - Button(String(localized: "pairing.connect.connectButton", defaultValue: "Connect")) { - Task { await store.connectManually() } - } - .disabled( - store.pairingTicketDraft.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty - || store.isConnecting - ) - } - Section(String(localized: "pairing.connect.section.status", defaultValue: "Status")) { LabeledContent(String(localized: "State", defaultValue: "State"), value: store.connectionBanner.label) // The observed network path stays visible even on this diff --git a/ios/ProgramaSpike/ProgramaSpike/Shared/Localizable.xcstrings b/ios/ProgramaSpike/ProgramaSpike/Shared/Localizable.xcstrings index da0f8e32..6a93aa20 100644 --- a/ios/ProgramaSpike/ProgramaSpike/Shared/Localizable.xcstrings +++ b/ios/ProgramaSpike/ProgramaSpike/Shared/Localizable.xcstrings @@ -783,23 +783,6 @@ } } }, - "pairing.connect.connectButton": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Connect" - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "接続" - } - } - } - }, "pairing.connect.error.invalidPastedCode": { "extractionState": "manual", "localizations": { @@ -902,23 +885,6 @@ } } }, - "pairing.connect.section.advanced": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Advanced: paste ticket and token separately" - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "詳細設定：チケットとトークンを個別に貼り付け" - } - } - } - }, "pairing.connect.section.code": { "extractionState": "manual", "localizations": { @@ -953,23 +919,6 @@ } } }, - "pairing.connect.ticketField.placeholder": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Paste the ticket from Programa's pairing screen" - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "Programaのペア設定画面からチケットを貼り付け" - } - } - } - }, "pairing.connect.title": { "extractionState": "manual", "localizations": { @@ -987,53 +936,19 @@ } } }, - "pairing.connect.tokenField.footnote": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Once this device is paired it stays trusted — you won't need the token again." - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "このデバイスは一度ペア設定されると信頼済みのままになり、次回からトークンは不要です。" - } - } - } - }, - "pairing.connect.tokenField.placeholder": { - "extractionState": "manual", - "localizations": { - "en": { - "stringUnit": { - "state": "translated", - "value": "Only needed the first time" - } - }, - "ja": { - "stringUnit": { - "state": "translated", - "value": "初回のみ必要です" - } - } - } - }, "pairing.connect.useCodeButton": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Use This Code" + "value": "Connect" } }, "ja": { "stringUnit": { "state": "translated", - "value": "このコードを使用" + "value": "接続" } } }