Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -54112,6 +54112,125 @@
}
}
},
"settings.browser.vscodeInlineSplitDirection": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Inline VS Code Placement"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "インライン VS Code の配置"
}
}
}
},
"settings.browser.vscodeInlineSplitDirection.bottom": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Bottom"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "下"
}
}
}
},
"settings.browser.vscodeInlineSplitDirection.left": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Left"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "左"
}
}
}
},
"settings.browser.vscodeInlineSplitDirection.right": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Right"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "右"
}
}
}
},
"settings.browser.vscodeInlineSplitDirection.subtitle": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Choose whether inline VS Code opens as a split or a tab, and where the split appears."
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "インライン VS Code を分割またはタブのどちらで開くかと、分割位置を選びます。"
}
}
}
},
"settings.browser.vscodeInlineSplitDirection.tab": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Tab"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "タブ"
}
}
}
},
"settings.browser.vscodeInlineSplitDirection.top": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Top"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "上"
}
}
}
},
"settings.browser.theme.subtitleForced": {
"extractionState": "manual",
"localizations": {
Expand Down
38 changes: 29 additions & 9 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1612,6 +1612,8 @@ struct ContentView: View {
private var commandPaletteRenameSelectAllOnFocus = CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus
@AppStorage(CommandPaletteSwitcherSearchSettings.searchAllSurfacesKey)
private var commandPaletteSearchAllSurfaces = CommandPaletteSwitcherSearchSettings.defaultSearchAllSurfaces
@AppStorage(VSCodeInlineSplitDirectionSettings.key)
private var vscodeInlineSplitDirectionRaw = VSCodeInlineSplitDirectionSettings.defaultDirection.rawValue
@AppStorage(BrowserLinkOpenSettings.openSidebarPullRequestLinksInCmuxBrowserKey)
private var openSidebarPullRequestLinksInCmuxBrowser = BrowserLinkOpenSettings.defaultOpenSidebarPullRequestLinksInCmuxBrowser
@FocusState private var isCommandPaletteSearchFocused: Bool
Expand Down Expand Up @@ -7337,6 +7339,11 @@ struct ContentView: View {
return openFocusedDirectory(directoryURL, in: target)
}

private var vscodeInlineSplitDirection: VSCodeInlineSplitDirection {
VSCodeInlineSplitDirectionSettings.parse(rawValue: vscodeInlineSplitDirectionRaw)
?? VSCodeInlineSplitDirectionSettings.defaultDirection
}

private func openFocusedDirectory(_ directoryURL: URL, in target: TerminalDirectoryOpenTarget) -> Bool {
switch target {
case .finder:
Expand All @@ -7355,10 +7362,12 @@ struct ContentView: View {
private func openFocusedDirectoryInInlineVSCode(_ directoryURL: URL) -> Bool {
guard let vscodeApplicationURL = TerminalDirectoryOpenTarget.vscodeInline.applicationURL(),
let workspace = tabManager.selectedWorkspace,
let sourcePanelId = workspace.focusedPanelId else {
let sourcePanelId = workspace.focusedPanelId,
let sourcePaneId = workspace.paneId(forPanelId: sourcePanelId) else {
return false
}
Comment on lines 7362 to 7367
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 sourcePaneId required unconditionally but only used in the tab branch

sourcePaneId is resolved in the guard clause and will cause an early false return if paneId(forPanelId:) comes back nil — but it is only consumed in the .tab branch below. For any of the four split directions (right, left, bottom, top), the function will silently fail via NSSound.beep() if paneId(forPanelId:) ever returns nil (e.g. because surfaceIdFromPanelId has no entry for the panel), even though those code paths never touch sourcePaneId.

This is a behavioural regression from the previous code, which only required sourcePanelId and had no dependency on pane lookup.

Consider resolving sourcePaneId lazily, only when the tab branch is actually taken:

guard let vscodeApplicationURL = TerminalDirectoryOpenTarget.vscodeInline.applicationURL(),
      let workspace = tabManager.selectedWorkspace,
      let sourcePanelId = workspace.focusedPanelId else {
    return false
}
let sourceTabId = workspace.id
let openDirection = vscodeInlineSplitDirection
let tabManager = tabManager
VSCodeServeWebController.shared.ensureServeWebURL(vscodeApplicationURL: vscodeApplicationURL) { serveWebURL in
    // ...
    let createdPanelId: UUID? = {
        if let splitDirection = openDirection.splitDirection {
            return tabManager.newBrowserSplit(
                tabId: sourceTabId,
                fromPanelId: sourcePanelId,
                orientation: splitDirection.orientation,
                insertFirst: splitDirection.insertFirst,
                url: openFolderURL,
                focus: true
            )
        }
        guard let sourcePaneId = workspace.paneId(forPanelId: sourcePanelId) else {
            NSSound.beep()
            return nil
        }
        return tabManager.newBrowserSurface(
            tabId: sourceTabId,
            inPane: sourcePaneId,
            url: openFolderURL,
            focus: true
        )
    }()
    // ...
}

let sourceTabId = workspace.id
let openDirection = vscodeInlineSplitDirection
let tabManager = tabManager
VSCodeServeWebController.shared.ensureServeWebURL(vscodeApplicationURL: vscodeApplicationURL) { serveWebURL in
guard let serveWebURL,
Expand All @@ -7369,14 +7378,25 @@ struct ContentView: View {
NSSound.beep()
return
}
guard tabManager.newBrowserSplit(
tabId: sourceTabId,
fromPanelId: sourcePanelId,
orientation: SplitDirection.right.orientation,
insertFirst: SplitDirection.right.insertFirst,
url: openFolderURL,
focus: true
) != nil else {
let createdPanelId: UUID? = {
if let splitDirection = openDirection.splitDirection {
return tabManager.newBrowserSplit(
tabId: sourceTabId,
fromPanelId: sourcePanelId,
orientation: splitDirection.orientation,
insertFirst: splitDirection.insertFirst,
url: openFolderURL,
focus: true
)
}
return tabManager.newBrowserSurface(
tabId: sourceTabId,
inPane: sourcePaneId,
url: openFolderURL,
focus: true
)
}()
guard createdPanelId != nil else {
NSSound.beep()
Comment on lines +7381 to 7403
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fall back to opening in the focused pane when split creation fails.

If newBrowserSplit(...) returns nil because the focused panel is not pane-backed, this still drops the inline VS Code open even though fallbackPaneId is already available. Falling back to newBrowserSurface(inPane:) here would keep left/right/top/bottom placements from failing unnecessarily.

Suggested fallback
             let createdPanelId: UUID? = {
-                if let splitDirection = openDirection.splitDirection {
-                    return tabManager.newBrowserSplit(
+                if let splitDirection = openDirection.splitDirection,
+                   let splitPanelId = tabManager.newBrowserSplit(
                         tabId: sourceTabId,
                         fromPanelId: sourcePanelId,
                         orientation: splitDirection.orientation,
                         insertFirst: splitDirection.insertFirst,
                         url: openFolderURL,
                         focus: true
-                    )
+                    ) {
+                    return splitPanelId
                 }
                 guard let sourcePaneId = workspace.paneId(forPanelId: sourcePanelId) ?? fallbackPaneId else {
                     return nil
                 }
                 return tabManager.newBrowserSurface(

Based on learnings, In Sources/ContentView.swift, openFileInTextEditor(_:) must attempt workspace.newTextEditorSplit(from:orientation:filePath:focus:) and, if that returns nil, fall back to workspace.newTextEditorSurface(inPane:filePath:focus:) using bonsplitController.focusedPaneId.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let createdPanelId: UUID? = {
if let splitDirection = openDirection.splitDirection {
return tabManager.newBrowserSplit(
tabId: sourceTabId,
fromPanelId: sourcePanelId,
orientation: splitDirection.orientation,
insertFirst: splitDirection.insertFirst,
url: openFolderURL,
focus: true
)
}
guard let sourcePaneId = workspace.paneId(forPanelId: sourcePanelId) ?? fallbackPaneId else {
return nil
}
return tabManager.newBrowserSurface(
tabId: sourceTabId,
inPane: sourcePaneId,
url: openFolderURL,
focus: true
)
}()
guard createdPanelId != nil else {
NSSound.beep()
let createdPanelId: UUID? = {
if let splitDirection = openDirection.splitDirection,
let splitPanelId = tabManager.newBrowserSplit(
tabId: sourceTabId,
fromPanelId: sourcePanelId,
orientation: splitDirection.orientation,
insertFirst: splitDirection.insertFirst,
url: openFolderURL,
focus: true
) {
return splitPanelId
}
guard let sourcePaneId = workspace.paneId(forPanelId: sourcePanelId) ?? fallbackPaneId else {
return nil
}
return tabManager.newBrowserSurface(
tabId: sourceTabId,
inPane: sourcePaneId,
url: openFolderURL,
focus: true
)
}()
guard createdPanelId != nil else {
NSSound.beep()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 7381 - 7403, When creating a new
browser panel, if tabManager.newBrowserSplit(...) returns nil (e.g., focused
panel isn't pane-backed) fall back to creating a surface in the focused pane
instead of bailing out; update the closure that computes createdPanelId so when
openDirection.splitDirection exists but newBrowserSplit returns nil you call
tabManager.newBrowserSurface(inPane: focusedPaneId, url: ..., focus: true) using
bonsplitController.focusedPaneId (or fallbackPaneId if needed), and similarly
when resolving sourcePaneId fallback to the focused pane before returning nil,
so newBrowserSurface is attempted whenever a split cannot be created.

return
}
Expand Down
11 changes: 11 additions & 0 deletions Sources/GhosttyConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ struct GhosttyConfig {
var sidebarBackgroundLight: NSColor?
var sidebarBackgroundDark: NSColor?
var sidebarTintOpacity: Double?
var vscodeInlineSplitDirection: VSCodeInlineSplitDirection?

// Palette colors (0-15)
var palette: [Int: NSColor] = [:]
Expand Down Expand Up @@ -189,6 +190,11 @@ struct GhosttyConfig {
}
}

func applyVSCodeInlineSplitDirectionToUserDefaults(defaults: UserDefaults = .standard) {
guard let vscodeInlineSplitDirection else { return }
defaults.set(vscodeInlineSplitDirection.rawValue, forKey: VSCodeInlineSplitDirectionSettings.key)
}

private static func loadFromDisk(preferredColorScheme: ColorSchemePreference) -> GhosttyConfig {
var config = GhosttyConfig()

Expand Down Expand Up @@ -218,6 +224,7 @@ struct GhosttyConfig {

config.resolveSidebarBackground(preferredColorScheme: preferredColorScheme)
config.applySidebarAppearanceToUserDefaults()
config.applyVSCodeInlineSplitDirectionToUserDefaults()

return config
}
Expand Down Expand Up @@ -304,6 +311,10 @@ struct GhosttyConfig {
if let opacity = Double(value) {
sidebarTintOpacity = min(max(opacity, 0), 1)
}
case VSCodeInlineSplitDirectionSettings.configKey:
if let direction = VSCodeInlineSplitDirectionSettings.parse(rawValue: value) {
vscodeInlineSplitDirection = direction
}
default:
break
}
Expand Down
74 changes: 74 additions & 0 deletions Sources/Panels/BrowserPanel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,80 @@ enum BrowserThemeSettings {
}
}

enum VSCodeInlineSplitDirection: String, CaseIterable, Identifiable {
case right
case left
case bottom
case top
case tab

var id: String { rawValue }

var displayName: String {
switch self {
case .right:
return String(localized: "settings.browser.vscodeInlineSplitDirection.right", defaultValue: "Right")
case .left:
return String(localized: "settings.browser.vscodeInlineSplitDirection.left", defaultValue: "Left")
case .bottom:
return String(localized: "settings.browser.vscodeInlineSplitDirection.bottom", defaultValue: "Bottom")
case .top:
return String(localized: "settings.browser.vscodeInlineSplitDirection.top", defaultValue: "Top")
case .tab:
return String(localized: "settings.browser.vscodeInlineSplitDirection.tab", defaultValue: "Tab")
}
}

var splitDirection: SplitDirection? {
switch self {
case .right:
return .right
case .left:
return .left
case .bottom:
return .down
case .top:
return .up
case .tab:
return nil
}
}
}

enum VSCodeInlineSplitDirectionSettings {
static let key = "vscodeInlineSplitDirection"
static let configKey = "vscode-inline-split-direction"
static let defaultDirection: VSCodeInlineSplitDirection = .right

static func parse(rawValue: String?) -> VSCodeInlineSplitDirection? {
guard let rawValue = rawValue?
.trimmingCharacters(in: .whitespacesAndNewlines)
.lowercased(),
!rawValue.isEmpty else {
return nil
}

switch rawValue {
case VSCodeInlineSplitDirection.right.rawValue:
return .right
case VSCodeInlineSplitDirection.left.rawValue:
return .left
case VSCodeInlineSplitDirection.bottom.rawValue, "down":
return .bottom
case VSCodeInlineSplitDirection.top.rawValue, "up":
return .top
case VSCodeInlineSplitDirection.tab.rawValue:
return .tab
default:
return nil
}
}

static func current(defaults: UserDefaults = .standard) -> VSCodeInlineSplitDirection {
parse(rawValue: defaults.string(forKey: key)) ?? defaultDirection
}
}

enum BrowserImportHintVariant: String, CaseIterable, Identifiable {
case inlineStrip
case floatingCard
Expand Down
4 changes: 3 additions & 1 deletion Sources/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3502,12 +3502,14 @@ class TabManager: ObservableObject {
tabId: UUID,
inPane paneId: PaneID,
url: URL? = nil,
preferredProfileID: UUID? = nil
preferredProfileID: UUID? = nil,
focus: Bool? = nil
) -> UUID? {
guard let tab = tabs.first(where: { $0.id == tabId }) else { return nil }
return tab.newBrowserSurface(
inPane: paneId,
url: url,
focus: focus,
preferredProfileID: preferredProfileID
)?.id
}
Expand Down
33 changes: 33 additions & 0 deletions Sources/cmuxApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3801,6 +3801,8 @@ struct SettingsView: View {
@AppStorage(BrowserSearchSettings.searchEngineKey) private var browserSearchEngine = BrowserSearchSettings.defaultSearchEngine.rawValue
@AppStorage(BrowserSearchSettings.searchSuggestionsEnabledKey) private var browserSearchSuggestionsEnabled = BrowserSearchSettings.defaultSearchSuggestionsEnabled
@AppStorage(BrowserThemeSettings.modeKey) private var browserThemeMode = BrowserThemeSettings.defaultMode.rawValue
@AppStorage(VSCodeInlineSplitDirectionSettings.key)
private var vscodeInlineSplitDirection = VSCodeInlineSplitDirectionSettings.defaultDirection.rawValue
@AppStorage(BrowserImportHintSettings.variantKey) private var browserImportHintVariantRaw = BrowserImportHintSettings.defaultVariant.rawValue
@AppStorage(BrowserImportHintSettings.showOnBlankTabsKey) private var showBrowserImportHintOnBlankTabs = BrowserImportHintSettings.defaultShowOnBlankTabs
@AppStorage(BrowserImportHintSettings.dismissedKey) private var isBrowserImportHintDismissed = BrowserImportHintSettings.defaultDismissed
Expand Down Expand Up @@ -3965,6 +3967,22 @@ struct SettingsView: View {
)
}

private var selectedVSCodeInlineSplitDirection: VSCodeInlineSplitDirection {
VSCodeInlineSplitDirectionSettings.parse(rawValue: vscodeInlineSplitDirection)
?? VSCodeInlineSplitDirectionSettings.defaultDirection
}

private var vscodeInlineSplitDirectionSelection: Binding<String> {
Binding(
get: { selectedVSCodeInlineSplitDirection.rawValue },
set: { newValue in
vscodeInlineSplitDirection =
(VSCodeInlineSplitDirectionSettings.parse(rawValue: newValue)
?? VSCodeInlineSplitDirectionSettings.defaultDirection).rawValue
}
)
}

private var browserImportHintVariant: BrowserImportHintVariant {
BrowserImportHintSettings.variant(for: browserImportHintVariantRaw)
}
Expand Down Expand Up @@ -5085,6 +5103,19 @@ struct SettingsView: View {

SettingsCardDivider()

SettingsPickerRow(
String(localized: "settings.browser.vscodeInlineSplitDirection", defaultValue: "Inline VS Code Placement"),
subtitle: String(localized: "settings.browser.vscodeInlineSplitDirection.subtitle", defaultValue: "Choose whether inline VS Code opens as a split or a tab, and where the split appears."),
controlWidth: pickerColumnWidth,
selection: vscodeInlineSplitDirectionSelection
) {
ForEach(VSCodeInlineSplitDirection.allCases) { direction in
Text(direction.displayName).tag(direction.rawValue)
}
}

SettingsCardDivider()

SettingsCardRow(
String(localized: "settings.browser.openTerminalLinks", defaultValue: "Open Terminal Links in cmux Browser"),
subtitle: String(localized: "settings.browser.openTerminalLinks.subtitle", defaultValue: "When off, links clicked in terminal output open in your default browser.")
Expand Down Expand Up @@ -5432,6 +5463,7 @@ struct SettingsView: View {
BrowserHistoryStore.shared.loadIfNeeded()
notificationStore.refreshAuthorizationStatus()
browserThemeMode = BrowserThemeSettings.mode(defaults: .standard).rawValue
vscodeInlineSplitDirection = VSCodeInlineSplitDirectionSettings.current(defaults: .standard).rawValue
browserImportHintVariantRaw = BrowserImportHintSettings.variant(for: browserImportHintVariantRaw).rawValue
browserHistoryEntryCount = BrowserHistoryStore.shared.entries.count
browserInsecureHTTPAllowlistDraft = browserInsecureHTTPAllowlist
Expand Down Expand Up @@ -5546,6 +5578,7 @@ struct SettingsView: View {
browserSearchEngine = BrowserSearchSettings.defaultSearchEngine.rawValue
browserSearchSuggestionsEnabled = BrowserSearchSettings.defaultSearchSuggestionsEnabled
browserThemeMode = BrowserThemeSettings.defaultMode.rawValue
vscodeInlineSplitDirection = VSCodeInlineSplitDirectionSettings.defaultDirection.rawValue
browserImportHintVariantRaw = BrowserImportHintSettings.defaultVariant.rawValue
showBrowserImportHintOnBlankTabs = BrowserImportHintSettings.defaultShowOnBlankTabs
isBrowserImportHintDismissed = BrowserImportHintSettings.defaultDismissed
Expand Down
Loading
Loading