From 038ebef16cc8cea570f87a95771eb76528935210 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 7 Mar 2026 07:04:17 -0800 Subject: [PATCH] address some PR feedback --- .../AppleScript/AppDelegate+AppleScript.swift | 48 ++++++++++--------- .../AppleScript/ScriptCloseCommand.swift | 26 +++++----- .../AppleScript/ScriptFocusCommand.swift | 14 +++--- .../AppleScript/ScriptSplitCommand.swift | 18 +++---- 4 files changed, 56 insertions(+), 50 deletions(-) diff --git a/macos/Sources/Features/AppleScript/AppDelegate+AppleScript.swift b/macos/Sources/Features/AppleScript/AppDelegate+AppleScript.swift index c1fba16e0..24309b348 100644 --- a/macos/Sources/Features/AppleScript/AppDelegate+AppleScript.swift +++ b/macos/Sources/Features/AppleScript/AppDelegate+AppleScript.swift @@ -115,7 +115,7 @@ extension NSApplication { /// /// We return a Bool to match the command's declared result type. @objc(handlePerformActionScriptCommand:) - func handlePerformActionScriptCommand(_ command: NSScriptCommand) -> Any? { + func handlePerformActionScriptCommand(_ command: NSScriptCommand) -> NSNumber? { guard validateScript(command: command) else { return nil } guard let action = command.directParameter as? String else { @@ -135,7 +135,7 @@ extension NSApplication { /// Handler for creating a reusable AppleScript surface configuration object. @objc(handleNewSurfaceConfigurationScriptCommand:) - func handleNewSurfaceConfigurationScriptCommand(_ command: NSScriptCommand) -> Any? { + func handleNewSurfaceConfigurationScriptCommand(_ command: NSScriptCommand) -> NSDictionary? { guard validateScript(command: command) else { return nil } do { @@ -159,7 +159,7 @@ extension NSApplication { /// /// Returns the newly created scripting window object. @objc(handleNewWindowScriptCommand:) - func handleNewWindowScriptCommand(_ command: NSScriptCommand) -> Any? { + func handleNewWindowScriptCommand(_ command: NSScriptCommand) -> ScriptWindow? { guard validateScript(command: command) else { return nil } guard let appDelegate = delegate as? AppDelegate else { @@ -168,15 +168,17 @@ extension NSApplication { return nil } - let baseConfig: Ghostty.SurfaceConfiguration - do { - baseConfig = try Ghostty.SurfaceConfiguration( - scriptRecord: command.evaluatedArguments?["configuration"] as? NSDictionary - ) - } catch { - command.scriptErrorNumber = errAECoercionFail - command.scriptErrorString = error.localizedDescription - return nil + let baseConfig: Ghostty.SurfaceConfiguration? + if let scriptRecord = command.evaluatedArguments?["configuration"] as? NSDictionary { + do { + baseConfig = try Ghostty.SurfaceConfiguration(scriptRecord: scriptRecord) + } catch { + command.scriptErrorNumber = errAECoercionFail + command.scriptErrorString = error.localizedDescription + return nil + } + } else { + baseConfig = nil } let controller = TerminalController.newWindow( @@ -205,7 +207,7 @@ extension NSApplication { /// /// Returns the newly created scripting tab object. @objc(handleNewTabScriptCommand:) - func handleNewTabScriptCommand(_ command: NSScriptCommand) -> Any? { + func handleNewTabScriptCommand(_ command: NSScriptCommand) -> ScriptTab? { guard validateScript(command: command) else { return nil } guard let appDelegate = delegate as? AppDelegate else { @@ -214,17 +216,17 @@ extension NSApplication { return nil } - let baseConfig: Ghostty.SurfaceConfiguration - do { - if let scriptRecord = command.evaluatedArguments?["configuration"] as? NSDictionary { + let baseConfig: Ghostty.SurfaceConfiguration? + if let scriptRecord = command.evaluatedArguments?["configuration"] as? NSDictionary { + do { baseConfig = try Ghostty.SurfaceConfiguration(scriptRecord: scriptRecord) - } else { - baseConfig = Ghostty.SurfaceConfiguration() + } catch { + command.scriptErrorNumber = errAECoercionFail + command.scriptErrorString = error.localizedDescription + return nil } - } catch { - command.scriptErrorNumber = errAECoercionFail - command.scriptErrorString = error.localizedDescription - return nil + } else { + baseConfig = nil } let targetWindow = command.evaluatedArguments?["window"] as? ScriptWindow @@ -285,7 +287,7 @@ extension NSApplication { @discardableResult func validateScript(command: NSScriptCommand) -> Bool { guard isAppleScriptEnabled else { - command.scriptErrorNumber = errAEEventFailed + command.scriptErrorNumber = errAEEventNotPermitted command.scriptErrorString = "AppleScript is disabled by the macos-applescript configuration." return false } diff --git a/macos/Sources/Features/AppleScript/ScriptCloseCommand.swift b/macos/Sources/Features/AppleScript/ScriptCloseCommand.swift index e62d9bb60..b38fb0e62 100644 --- a/macos/Sources/Features/AppleScript/ScriptCloseCommand.swift +++ b/macos/Sources/Features/AppleScript/ScriptCloseCommand.swift @@ -34,7 +34,8 @@ final class ScriptCloseCommand: NSScriptCommand { } } -/// Handler for the `close tab` AppleScript command defined in `Ghostty.sdef`. +/// Handler for the container-level `close tab` AppleScript command defined in +/// `Ghostty.sdef`. @MainActor @objc(GhosttyScriptCloseTabCommand) final class ScriptCloseTabCommand: NSScriptCommand { @@ -47,29 +48,30 @@ final class ScriptCloseTabCommand: NSScriptCommand { return nil } - guard let controller = tab.parentController else { + guard let tabController = tab.parentController else { scriptErrorNumber = errAEEventFailed scriptErrorString = "Tab is no longer available." return nil } - if let terminalController = controller as? TerminalController { - terminalController.closeTabImmediately(registerRedo: false) + if let managedTerminalController = tabController as? TerminalController { + managedTerminalController.closeTabImmediately(registerRedo: false) return nil } - guard let targetWindow = tab.parentWindow else { + guard let tabContainerWindow = tab.parentWindow else { scriptErrorNumber = errAEEventFailed - scriptErrorString = "Tab window is no longer available." + scriptErrorString = "Tab container window is no longer available." return nil } - targetWindow.close() + tabContainerWindow.close() return nil } } -/// Handler for the `close window` AppleScript command defined in `Ghostty.sdef`. +/// Handler for the container-level `close window` AppleScript command defined in +/// `Ghostty.sdef`. @MainActor @objc(GhosttyScriptCloseWindowCommand) final class ScriptCloseWindowCommand: NSScriptCommand { @@ -82,18 +84,18 @@ final class ScriptCloseWindowCommand: NSScriptCommand { return nil } - if let terminalController = window.preferredController as? TerminalController { - terminalController.closeWindowImmediately() + if let managedTerminalController = window.preferredController as? TerminalController { + managedTerminalController.closeWindowImmediately() return nil } - guard let targetWindow = window.preferredParentWindow else { + guard let windowContainer = window.preferredParentWindow else { scriptErrorNumber = errAEEventFailed scriptErrorString = "Window is no longer available." return nil } - targetWindow.close() + windowContainer.close() return nil } } diff --git a/macos/Sources/Features/AppleScript/ScriptFocusCommand.swift b/macos/Sources/Features/AppleScript/ScriptFocusCommand.swift index 61b4ab8c6..0be3aaf69 100644 --- a/macos/Sources/Features/AppleScript/ScriptFocusCommand.swift +++ b/macos/Sources/Features/AppleScript/ScriptFocusCommand.swift @@ -34,7 +34,8 @@ final class ScriptFocusCommand: NSScriptCommand { } } -/// Handler for the `activate window` AppleScript command defined in `Ghostty.sdef`. +/// Handler for the container-level `activate window` AppleScript command +/// defined in `Ghostty.sdef`. @MainActor @objc(GhosttyScriptActivateWindowCommand) final class ScriptActivateWindowCommand: NSScriptCommand { @@ -47,19 +48,20 @@ final class ScriptActivateWindowCommand: NSScriptCommand { return nil } - guard let targetWindow = window.preferredParentWindow else { + guard let windowContainer = window.preferredParentWindow else { scriptErrorNumber = errAEEventFailed scriptErrorString = "Window is no longer available." return nil } - targetWindow.makeKeyAndOrderFront(nil) + windowContainer.makeKeyAndOrderFront(nil) NSApp.activate(ignoringOtherApps: true) return nil } } -/// Handler for the `select tab` AppleScript command defined in `Ghostty.sdef`. +/// Handler for the container-level `select tab` AppleScript command defined in +/// `Ghostty.sdef`. @MainActor @objc(GhosttyScriptSelectTabCommand) final class ScriptSelectTabCommand: NSScriptCommand { @@ -72,13 +74,13 @@ final class ScriptSelectTabCommand: NSScriptCommand { return nil } - guard let targetWindow = tab.parentWindow else { + guard let tabContainerWindow = tab.parentWindow else { scriptErrorNumber = errAEEventFailed scriptErrorString = "Tab is no longer available." return nil } - targetWindow.makeKeyAndOrderFront(nil) + tabContainerWindow.makeKeyAndOrderFront(nil) NSApp.activate(ignoringOtherApps: true) return nil } diff --git a/macos/Sources/Features/AppleScript/ScriptSplitCommand.swift b/macos/Sources/Features/AppleScript/ScriptSplitCommand.swift index 30f34fc8e..b3ab0fa73 100644 --- a/macos/Sources/Features/AppleScript/ScriptSplitCommand.swift +++ b/macos/Sources/Features/AppleScript/ScriptSplitCommand.swift @@ -30,17 +30,17 @@ final class ScriptSplitCommand: NSScriptCommand { return nil } - let baseConfig: Ghostty.SurfaceConfiguration - do { - if let scriptRecord = evaluatedArguments?["configuration"] as? NSDictionary { + let baseConfig: Ghostty.SurfaceConfiguration? + if let scriptRecord = evaluatedArguments?["configuration"] as? NSDictionary { + do { baseConfig = try Ghostty.SurfaceConfiguration(scriptRecord: scriptRecord) - } else { - baseConfig = Ghostty.SurfaceConfiguration() + } catch { + scriptErrorNumber = errAECoercionFail + scriptErrorString = error.localizedDescription + return nil } - } catch { - scriptErrorNumber = errAECoercionFail - scriptErrorString = error.localizedDescription - return nil + } else { + baseConfig = nil } // Find the window controller that owns this surface.