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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 71 additions & 4 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5473,7 +5473,32 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
}

@objc func openNewMainWindow(_ sender: Any?) {
_ = createMainWindow()
_ = createMainWindow(sourceWindow: preferredSourceWindowForNewMainWindow(sender: sender))
}

func openNewMainWindow(preferredWindow: NSWindow?) {
_ = createMainWindow(sourceWindow: preferredWindow)
}

private func preferredSourceWindowForNewMainWindow(sender: Any?) -> NSWindow? {
if let window = sender as? NSWindow, isMainTerminalWindow(window) {
return window
}
if let event = NSApp.currentEvent,
let window = mainWindowForShortcutEvent(event) {
return window
}
if let keyWindow = NSApp.keyWindow, isMainTerminalWindow(keyWindow) {
return keyWindow
}
if let mainWindow = NSApp.mainWindow, isMainTerminalWindow(mainWindow) {
return mainWindow
}
if let context = preferredRegisteredMainWindowContext(),
let window = resolvedWindow(for: context) {
return window
}
return nil
}

func scheduleInitialMainWindowBootstrap(debugSource: String) {
Expand Down Expand Up @@ -6177,11 +6202,50 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return true
}

private func resolvedMainWindowSource(_ window: NSWindow?) -> NSWindow? {
guard let window else { return nil }
if isMainTerminalWindow(window) {
return window
}
if let context = contextForMainWindow(window) ?? contextForMainTerminalWindow(window) {
return resolvedWindow(for: context)
}
return nil
}

private func positionNewMainWindow(_ window: NSWindow, relativeTo sourceWindow: NSWindow) {
let sourceFrame = sourceWindow.frame
let sourceScreen = sourceWindow.screen
?? NSScreen.screens.first(where: { $0.frame.intersects(sourceFrame) })
guard let visibleFrame = sourceScreen?.visibleFrame else {
window.center()
return
}

let cascadeOffset: CGFloat = 24
let minimumWindowSize = NSSize(width: 460, height: 360)
var frame = window.frame
frame.origin = NSPoint(
x: sourceFrame.minX + cascadeOffset,
y: sourceFrame.maxY - cascadeOffset - frame.height
)
window.setFrame(
Self.clampFrame(
frame,
within: visibleFrame,
minWidth: minimumWindowSize.width,
minHeight: minimumWindowSize.height
),
display: false
)
}

@discardableResult
func createMainWindow(
initialWorkingDirectory: String? = nil,
sessionWindowSnapshot: SessionWindowSnapshot? = nil,
shouldActivate: Bool = true
shouldActivate: Bool = true,
sourceWindow preferredSourceWindow: NSWindow? = nil
) -> UUID {
let windowId = UUID()
let tabManager = TabManager(initialWorkingDirectory: initialWorkingDirectory)
Expand Down Expand Up @@ -6235,7 +6299,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
let sourceContext = preferredMainWindowContextForWorkspaceCreation(
debugSource: "createMainWindow.initialGeometry"
)
let sourceWindow = sourceContext.flatMap { resolvedWindow(for: $0) }
let sourceWindow = resolvedMainWindowSource(preferredSourceWindow)
?? sourceContext.flatMap { resolvedWindow(for: $0) }
let existingFrame = sourceWindow?.frame
let sourceWindowIsNativeFullScreen: Bool = {
#if DEBUG
Expand Down Expand Up @@ -6279,6 +6344,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
let restoredFrame = resolvedWindowFrame(from: sessionWindowSnapshot)
if let restoredFrame {
window.setFrame(restoredFrame, display: false)
} else if let sourceWindow {
positionNewMainWindow(window, relativeTo: sourceWindow)
} else {
window.center()
// Cascade using the same algorithm as upstream Ghostty: seed from
Expand Down Expand Up @@ -10299,7 +10366,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
// after a browser panel has been shown, SwiftUI's menu dispatch can silently
// consume the key equivalent without firing the action closure.
if matchConfiguredShortcut(event: event, action: .newWindow) {
openNewMainWindow(nil)
openNewMainWindow(preferredWindow: mainWindowForShortcutEvent(event))
return true
}

Expand Down
3 changes: 2 additions & 1 deletion Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7683,7 +7683,8 @@ struct ContentView: View {
}
}
registry.register(commandId: "palette.newWindow") {
AppDelegate.shared?.openNewMainWindow(nil)
guard let appDelegate = AppDelegate.shared else { return }
appDelegate.openNewMainWindow(preferredWindow: appDelegate.mainWindow(for: windowId))
}
Comment on lines 7685 to 7688

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.

P2 Double optional-chain on AppDelegate.shared

AppDelegate.shared is dereferenced twice: once for the call site and again inside the argument expression. Because AppDelegate.shared is a singleton the double dereference is safe, but it reads as if the inner call could diverge. Capturing the delegate once is cleaner:

registry.register(commandId: "palette.newWindow") {
    guard let delegate = AppDelegate.shared else { return }
    delegate.openNewMainWindow(preferredWindow: delegate.mainWindow(for: windowId))
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed by capturing AppDelegate.shared once in the palette command and using that delegate for both the lookup and the new-window call.

— Claude Code

registry.register(commandId: "palette.installCLI") {
AppDelegate.shared?.installCmuxCLIInPath(nil)
Expand Down
87 changes: 87 additions & 0 deletions cmuxTests/AppDelegateShortcutRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,93 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
createdWindowId = newWindowIds.first
}

func testCmdShiftNCreatesWindowFromEventWindowWithoutAddingWorkspace() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

let firstWindowId = appDelegate.createMainWindow()
let secondWindowId = appDelegate.createMainWindow()
var createdWindowId: UUID?

defer {
if let createdWindowId {
closeWindow(withId: createdWindowId)
}
closeWindow(withId: firstWindowId)
closeWindow(withId: secondWindowId)
}

guard let firstManager = appDelegate.tabManagerFor(windowId: firstWindowId),
let secondManager = appDelegate.tabManagerFor(windowId: secondWindowId),
let firstWindow = window(withId: firstWindowId),
let secondWindow = window(withId: secondWindowId),
let visibleFrame = (secondWindow.screen ?? NSScreen.main)?.visibleFrame else {
XCTFail("Expected both window contexts to exist")
return
}

let firstFrame = NSRect(
x: visibleFrame.minX + 40,
y: visibleFrame.maxY - 460,
width: 760,
height: 420
)
let secondFrame = NSRect(
x: min(visibleFrame.minX + 180, visibleFrame.maxX - 600),
y: max(visibleFrame.minY + 80, visibleFrame.maxY - 560),
width: 560,
height: 380
)
firstWindow.setFrame(firstFrame, display: true)
secondWindow.setFrame(secondFrame, display: true)
firstWindow.makeKeyAndOrderFront(nil)
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))

let eventSourceFrame = secondWindow.frame
let firstCount = firstManager.tabs.count
let secondCount = secondManager.tabs.count
let existingWindowIds = mainWindowIds()

guard let event = makeKeyDownEvent(
key: "n",
modifiers: [.command, .shift],
keyCode: 45,
windowNumber: secondWindow.windowNumber
) else {
XCTFail("Failed to construct Cmd+Shift+N event")
return
}

#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))

let newWindowIds = mainWindowIds().subtracting(existingWindowIds)
XCTAssertEqual(newWindowIds.count, 1, "Cmd+Shift+N should create one new main window")
createdWindowId = newWindowIds.first

XCTAssertEqual(firstManager.tabs.count, firstCount, "Cmd+Shift+N must not create a workspace in the key window")
XCTAssertEqual(secondManager.tabs.count, secondCount, "Cmd+Shift+N must not create a workspace in the event window")

guard let createdWindowId,
let createdWindow = window(withId: createdWindowId) else {
XCTFail("Expected created window")
return
}

XCTAssertEqual(createdWindow.frame.width, eventSourceFrame.width, accuracy: 1)
XCTAssertEqual(createdWindow.frame.height, eventSourceFrame.height, accuracy: 1)
XCTAssertTrue(
visibleFrame.contains(createdWindow.frame),
"New window should be placed inside the source window display"
)
}

func testAddWorkspaceInPreferredMainWindowUsesKeyWindowWhenObjectKeyLookupIsMismatched() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
Expand Down
Loading