Fix embedded surface updates during pane strip motion#15
Open
lawrencecchen wants to merge 608 commits intomainfrom
Open
Fix embedded surface updates during pane strip motion#15lawrencecchen wants to merge 608 commits intomainfrom
lawrencecchen wants to merge 608 commits intomainfrom
Conversation
Following the discussion at ghostty-org#10852, I believe this is the right default. I'm willing to continue to revisit this decisions, but Ghostty 1.3 is around the corner and I don't think such a change like this should be pushed into it. I think palette generation is best left as a _theme author_ tool. A Ghostty color theme could include `palette-generate=true` if it wants to customize the 256-color palette more easily. Of course, end users can as well anytime. Another part of my reasoning is that TUI programs who want this behavior can already achieve it themselves by mixing dark/light theme detection via CSI 996 (https://contour-terminal.org/vt-extensions/color-palette-update-notifications/) with OSC 4/10/11 color query and change sequences, both of which are decently supported in the terminal ecosystem and fully supported in Ghostty. I'm also open to considering some kind of new sequence to make this easier for TUIs (probably a mode) where they can opt-in to palette generation plus "harmonius" palettes (see `palette-harmonius`) and Ghostty does it on demand then. I think that'd solve the legacy vs new TUI argument where legacy programs can continue to make assumptions about the palette and new programs can opt-in to a more dynamic palette without having to do a lot of work themselves.
Following the discussion at ghostty-org#10852, I believe this is the right default. I'm willing to continue to revisit this decisions, but Ghostty 1.3 is around the corner and I don't think such a change like this should be pushed into it. This was proposed before but I wanted to wait to iron out any bugs here and I think we have. Namely we identified one bug where we were accidentally overriding our _default_ palette which shouldn't happen. But now that it has sat awhile and we've gathered enough feedback, I'm willing to commit to it. In general, we got **very little negative feedback.** The linked discussion shows very little activity relative to other more controversial changes we've made. It has basically 1 upvote with around 5 participants whereas our most popular or breaking features/bugs have had at least dozens if not over a hundred. I think this shows that this change isn't that disruptive, either because the colors work fine or because not that many things use the 256-color palette (probably the latter moreso but a mix for sure). For that reason, I do think revisiting this at some point is warranted. I think palette generation is best left as a _theme author_ tool. A Ghostty color theme could include `palette-generate=true` if it wants to customize the 256-color palette more easily. Of course, end users can as well anytime. Another part of my reasoning is that TUI programs who want this behavior can already achieve it themselves by mixing dark/light theme detection via CSI 996 (https://contour-terminal.org/vt-extensions/color-palette-update-notifications/) with OSC 4/10/11 color query and change sequences, both of which are decently supported in the terminal ecosystem and fully supported in Ghostty. I'm also open to considering some kind of new sequence to make this easier for TUIs (probably a mode) where they can opt-in to palette generation plus "harmonius" palettes (see `palette-harmonius`) and Ghostty does it on demand then. I think that'd solve the legacy vs new TUI argument where legacy programs can continue to make assumptions about the palette and new programs can opt-in to a more dynamic palette without having to do a lot of work themselves. cc @jake-stewart
“always include a language and a country code” reads as “always include a language, and also always include a country code”, while the intended meaning was that it includes both a language *code* and a country code.
That line was intended to guide those who do not normally edit po files with a plain text editor, but ended up sounding like it states the obvious (“to do X, do X”) before this change.
This fixes two overlapping issues regarding window positioning and Cmd+W window closures on macOS: 1. `window-position-x` and `window-position-y` coordinates were being ignored on initial launch because `TerminalWindow.setInitialWindowPosition` depended on the `TerminalController`, which isn't fully attached during `awakeFromNib`. This logic was moved so explicit coordinates are correctly enforced. 2. When closing a window via Cmd+W (leaving the app active), reopening the window would continuously cascade down and to the right rather than restoring to the previous position. It now checks if there are other windows open before cascading. 3. `LastWindowPosition` was updated to save both the frame origin and size (width/height), ensuring that restoring a closed window correctly mimics native AppKit State Restoration size behaviors while honoring explicit configurations.
Triggered by [comment](ghostty-org#11070 (comment)) from @mitchellh. Vouch: @abdurrahmanski Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Portugal exists! Wikipedia notes [1] it to be the main other dialect. There's already a PR for pt_PT support too: ghostty-org#9078. It was renamed from pt_BR.UTF-8.po to pt.po in ghostty-org#10976. [1]: https://en.wikipedia.org/wiki/Portuguese_dialects
Follow-up to ghostty-org#10976. Portugal exists! [Wikipedia notes it] to be the main other dialect. There's already a PR for `pt_PT` support too: ghostty-org#9078. [Wikipedia notes it]: https://en.wikipedia.org/wiki/Portuguese_dialects
While it was renamed from ko_KR.UTF-8.po to ko.po in ghostty-org#10976, @uhojin, a Korean locale maintainer, notes [1] that “ko_KR [*South* Korean] makes more sense in locale context just to avoid any potential confusion between 한국어 vs 조선어”. Despite ko_KP (North Korean) not being present in glibc (as of version 2.43), and the ISO639 maintainers expressing disapproval of ko_KP [2], it is possible opinions may change in the future, and individual opinions may be contested—disambiguating doesn't hurt. [1]: ghostty-org#10976 (comment) [2]: ghostty-org#10976 (comment)
While it was renamed from `ko_KR.UTF-8.po` to `ko.po` in ghostty-org#10976, @uhojin, a Korean locale maintainer, [notes] that “ko_KR [*South* Korean] makes more sense in locale context just to avoid any potential confusion between 한국어 vs 조선어”. Despite `ko_KP` (North Korean) not being present in glibc (as of version 2.43), and the ISO639 maintainers [expressing disapproval of `ko_KP`], it is possible opinions may change in the future, and individual opinions may be contested—disambiguating doesn't hurt. Requesting a review from all involved parties; I wish you ***all* opine before merging**. [notes]: ghostty-org#10976 (comment) [expressing disapproval of `ko_KP`]: ghostty-org#10976 (comment)
Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
Triggered by [discussion comment](ghostty-org#11076 (comment)) from @jcollie. Vouch: @DiaaEddin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
apply reviewer suggestion for cascading Co-authored-by: Lukas <134181853+bo2themax@users.noreply.github.com>
) I'd like to contribute a fix for an issue I found regarding how macOS window restoration works when a window is closed via Cmd+W (leaving the app active). Currently, the position cascades down and to the right on every reopen, and size explicitly resets. Also, explicit `window-position-x/y` configs get ignored on first launch. I've diagnosed the issues: 1. In `TerminalWindow.swift`, `setInitialWindowPosition` relies on the `TerminalController` which isn't present during `awakeFromNib`. I moved the `screen.origin` calculation directly into the window class to ensure fixed coordinates are respected immediately. 2. In `TerminalController.swift`, I consolidated the window spawning cascade logic into a new `applyCascade(to:hasFixedPos:)` helper. It now only calls `cascadeTopLeft` if `TerminalController.all.count > 1` (meaning another window is active) and fixed coords aren't set. If it's the only window, it anchors exactly where `LastWindowPosition` placed it. 3. In `LastWindowPosition.swift`, I updated the `save` and `restore` logic to persist the full `window.frame` (origin + size) instead of just the origin. *Disclosure: I used Cursor (Tab) to assist in navigating the codebase and formatting the Swift code, but I fully understand the AppKit lifecycle changes and edge cases I'm proposing.* I have the commit locally formatted with `swiftlint` and ready to push!
We haven't used or run these in forever (literally like 3+ years). They're just wasting cognitive space and confuse some users as to what they're for. Remove them.
We haven't used or run these in forever (literally like 3+ years). They're just wasting cognitive space and confuse some users as to what they're for. Remove them.
When a DCS sequence has more than MAX_PARAMS parameters, entering dcs_passthrough would write to params[params_idx] without a bounds check, causing an out-of-bounds access. Drop the entire DCS hook when params overflow, consistent with how csi_dispatch handles it. Found by AFL fuzzing.
…1088) When a DCS sequence has more than MAX_PARAMS parameters, entering dcs_passthrough would write to params[params_idx] without a bounds check, causing an out-of-bounds access. Add the same guard that csi_dispatch already has. Found by AFL fuzzing, test and fix produced by Codex.
Remove the stale GHOSTTY_SGR_ATTR_RESET_UNDERLINE entry from the C header and renumber subsequent GhosttySgrAttributeTag values to match src/terminal/sgr.zig Attribute.Tag ordering. This fixes misclassified attributes from ghostty_sgr_next for C consumers that switch on the enum tags from include/ghostty/vt/sgr.h.
If you have multiple splits and start searching naturally the focus transfers over to the search widget which would apply the unfocused options. This could make it difficult to view your matches from searching without re-focusing the surface. This was discovered when I tested ghostty-org#11218 (which is a different issue)
Triggered by [discussion comment](ghostty-org#11246 (comment)) from @jcollie. Vouch: @jmcgover Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This adds two new propeties to make it easy to get the frontmost (main) window and the focused terminal within a tab. We already had a property to get the selected tab of a tab group.
…hostty-org#11251) This adds two new propeties to make it easy to get the frontmost (main) window and the focused terminal within a tab. We already had a property to get the selected tab of a tab group. ## Examples ### Send Input to Focused Terminal ```AppleScript tell application "Ghostty" set term to focused terminal of selected tab of front window input text "pwd\n" to term end tell ``` ### Split the Focused Terminal ```applescript tell application "Ghostty" set currentTerm to focused terminal of selected tab of front window set newTerm to split currentTerm direction right input text "echo split-ready\n" to newTerm end tell ```
…for rendering." (ghostty-org#11227) - "Unable to acquire an OpenGL context for rendering." This could be translated to "No se puede" or "No se pudo", depends on the context of the message. If the message is showing a current intent the translation should be "No se puede", if the message is communicating that Ghostty failed to acquire the OpenGL then the translation should be "No se pudo", here I need more context. Either case the wording "No se puedo" is incorrect.
This fixes an error if the script was sourced a second time:
bash: __ghostty_ps0: readonly variable
Because this is a non-exported variable, this would only happen if the
script was sourced multiple times in the same bash session.
If an existing PROMPT_COMMAND was a string ending in ; (and maybe some spaces), we'd add a redundant ;, resulting in a syntax error. Now we strip any trailing `;[[:space:]]*` characters from the original string before add ours.
If an existing PROMPT_COMMAND was a string ending in ; (and maybe some spaces), we'd add a redundant ;, resulting in a syntax error. Now we strip any trailing `;[[:space:]]*` characters from the original string before add ours. Fixes ghostty-org#11259
This fixes an error if the script was sourced a second time:
bash: __ghostty_ps0: readonly variable
Because this is a non-exported variable, this would only happen if the
script was sourced multiple times in the same bash session.
When a grapheme expands to width 2 at the screen edge, this path can write spacer_head before printWrap() sets row.wrap. With an active hyperlink, printCell triggers hyperlink bookkeeping and page integrity checks in that intermediate state, causing UnwrappedSpacerHead. Mark row.wrap before writing spacer_head in this grapheme-wrap path to keep the intermediate state valid.
…rg#11264) When a grapheme expands to width 2 at the screen edge, this path can write spacer_head before printWrap() sets row.wrap. With an active hyperlink, printCell triggers hyperlink bookkeeping and page integrity checks in that intermediate state, causing UnwrappedSpacerHead. Mark row.wrap before writing spacer_head in this grapheme-wrap path to keep the intermediate state valid.
…ostty-org#11257) Fixes ghostty-org#11256, which is rather hard to reproduce on macOS 26, but after adding breaking points on size update, we can see that it happens when the `intrinsicContentSize` is not properly updated. <img width="998" height="556" alt="Xnip2026-03-09_11-38-40" src="https://github.com/user-attachments/assets/8ac1de91-5895-45fc-a443-002eb016a1ce" />
…ndow (ghostty-org#11257)" This reverts commit 3445c9a, reversing changes made to 1e981f8.
Band-aid for ghostty-org#10304 We don't have a robust fix yet but this should help mitigate more scenarios.
…ostty-org#11265) Band-aid for ghostty-org#10304 We don't have a robust fix yet but this should help mitigate more scenarios.
Triggered by [discussion comment](ghostty-org#11274 (comment)) from @jcollie. Vouch: @seruman Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 3000 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="macos/AGENTS.md">
<violation number="1" location="macos/AGENTS.md:28">
P2: The AppleScript example path is inconsistent with the documented macOS app output directory; use `macos/build/Debug/Ghostty.app` to avoid targeting a non-existent bundle path.</violation>
</file>
<file name="macos/Sources/Features/About/AboutViewModel.swift">
<violation number="1" location="macos/Sources/Features/About/AboutViewModel.swift:36">
P2: When `currentIcon` is nil, the first advance skips the first icon (`.official`) due to the `?? 0` fallback before calling `indexWrapping(after:)`.</violation>
</file>
<file name="macos/Sources/Features/AppleScript/ScriptSurfaceConfiguration.swift">
<violation number="1" location="macos/Sources/Features/AppleScript/ScriptSurfaceConfiguration.swift:79">
P2: Invalid `environmentVariables` input types are silently ignored instead of producing a parse error.</violation>
</file>
<file name="macos/Sources/Features/Custom App Icon/DockTilePlugin.swift">
<violation number="1" location="macos/Sources/Features/Custom App Icon/DockTilePlugin.swift:143">
P2: Avoid declaring `NSDockTile` as `@unchecked Sendable`. This masks thread-safety issues for a main-thread-only AppKit type; instead keep dock tile updates on the main thread and remove the unchecked conformance.</violation>
</file>
<file name="macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift">
<violation number="1" location="macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift:89">
P2: Center initial Y origin uses `window.frame.width` and omits `visibleFrame.origin.y`, causing incorrect vertical placement during center transitions.</violation>
</file>
<file name="macos/Sources/Features/AppleScript/AppDelegate+AppleScript.swift">
<violation number="1" location="macos/Sources/Features/AppleScript/AppDelegate+AppleScript.swift:291">
P2: The `new tab` fallback can return a tab object anchored to a transient window ID. If tab-group state hasn’t settled yet, later AppleScript resolution of that returned tab may fail.</violation>
</file>
<file name="macos/Sources/App/macOS/AppDelegate.swift">
<violation number="1" location="macos/Sources/App/macOS/AppDelegate.swift:782">
P2: The local `NSSound` instance will be deallocated by ARC before the asynchronous playback completes, causing the custom bell audio to cut off or fail.</violation>
</file>
<file name="macos/Ghostty.xcodeproj/project.pbxproj">
<violation number="1" location="macos/Ghostty.xcodeproj/project.pbxproj:413">
P1: DockTilePlugin target has no synchronized source group membership, leaving its Sources phase empty and producing a plugin bundle without the plugin implementation.</violation>
<violation number="2" location="macos/Ghostty.xcodeproj/project.pbxproj:869">
P1: `NSPrincipalClass` is empty for DockTilePlugin, so the dock tile plugin class is not declared as the bundle entry point.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ); | ||
| dependencies = ( | ||
| ); | ||
| name = DockTilePlugin; |
There was a problem hiding this comment.
P1: DockTilePlugin target has no synchronized source group membership, leaving its Sources phase empty and producing a plugin bundle without the plugin implementation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Ghostty.xcodeproj/project.pbxproj, line 413:
<comment>DockTilePlugin target has no synchronized source group membership, leaving its Sources phase empty and producing a plugin bundle without the plugin implementation.</comment>
<file context>
@@ -328,6 +398,25 @@
+ );
+ dependencies = (
+ );
+ name = DockTilePlugin;
+ packageProductDependencies = (
+ );
</file context>
| GENERATE_INFOPLIST_FILE = YES; | ||
| INFOPLIST_KEY_CFBundleDisplayName = "Ghostty Dock Tile Plugin"; | ||
| INFOPLIST_KEY_NSHumanReadableCopyright = ""; | ||
| INFOPLIST_KEY_NSPrincipalClass = ""; |
There was a problem hiding this comment.
P1: NSPrincipalClass is empty for DockTilePlugin, so the dock tile plugin class is not declared as the bundle entry point.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Ghostty.xcodeproj/project.pbxproj, line 869:
<comment>`NSPrincipalClass` is empty for DockTilePlugin, so the dock tile plugin class is not declared as the bundle entry point.</comment>
<file context>
@@ -712,6 +854,93 @@
+ GENERATE_INFOPLIST_FILE = YES;
+ INFOPLIST_KEY_CFBundleDisplayName = "Ghostty Dock Tile Plugin";
+ INFOPLIST_KEY_NSHumanReadableCopyright = "";
+ INFOPLIST_KEY_NSPrincipalClass = "";
+ INSTALL_PATH = "$(LOCAL_LIBRARY_DIR)/Bundles";
+ LOCALIZATION_PREFERS_STRING_CATALOGS = YES;
</file context>
| (1) Build with `macos/build.nu` | ||
| (2) Launch and activate the app via osascript using the absolute path | ||
| to the built app bundle: | ||
| `osascript -e 'tell application "<absolute path to build/Debug/Ghostty.app>" to activate'` |
There was a problem hiding this comment.
P2: The AppleScript example path is inconsistent with the documented macOS app output directory; use macos/build/Debug/Ghostty.app to avoid targeting a non-existent bundle path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/AGENTS.md, line 28:
<comment>The AppleScript example path is inconsistent with the documented macOS app output directory; use `macos/build/Debug/Ghostty.app` to avoid targeting a non-existent bundle path.</comment>
<file context>
@@ -0,0 +1,34 @@
+ (1) Build with `macos/build.nu`
+ (2) Launch and activate the app via osascript using the absolute path
+ to the built app bundle:
+ `osascript -e 'tell application "<absolute path to build/Debug/Ghostty.app>" to activate'`
+ (3) Wait a few seconds for the app to fully launch and open a terminal.
+ (4) Run test scripts with `osascript`, always targeting the app by
</file context>
| } | ||
|
|
||
| func advanceToNextIcon() { | ||
| let currentIndex = currentIcon.flatMap(icons.firstIndex(of:)) ?? 0 |
There was a problem hiding this comment.
P2: When currentIcon is nil, the first advance skips the first icon (.official) due to the ?? 0 fallback before calling indexWrapping(after:).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Sources/Features/About/AboutViewModel.swift, line 36:
<comment>When `currentIcon` is nil, the first advance skips the first icon (`.official`) due to the `?? 0` fallback before calling `indexWrapping(after:)`.</comment>
<file context>
@@ -0,0 +1,40 @@
+ }
+
+ func advanceToNextIcon() {
+ let currentIndex = currentIcon.flatMap(icons.firstIndex(of:)) ?? 0
+ let nextIndex = icons.indexWrapping(after: currentIndex)
+ currentIcon = icons[nextIndex]
</file context>
| } | ||
| } | ||
|
|
||
| if let assignments = raw["environmentVariables"] as? [String], !assignments.isEmpty { |
There was a problem hiding this comment.
P2: Invalid environmentVariables input types are silently ignored instead of producing a parse error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Sources/Features/AppleScript/ScriptSurfaceConfiguration.swift, line 79:
<comment>Invalid `environmentVariables` input types are silently ignored instead of producing a parse error.</comment>
<file context>
@@ -0,0 +1,140 @@
+ }
+ }
+
+ if let assignments = raw["environmentVariables"] as? [String], !assignments.isEmpty {
+ environmentVariables = try Self.parseScriptEnvironmentAssignments(assignments)
+ }
</file context>
|
|
||
| // This is required because of the DispatchQueue call above. This doesn't | ||
| // feel right but I don't know a better way to solve this. | ||
| extension NSDockTile: @unchecked @retroactive Sendable {} |
There was a problem hiding this comment.
P2: Avoid declaring NSDockTile as @unchecked Sendable. This masks thread-safety issues for a main-thread-only AppKit type; instead keep dock tile updates on the main thread and remove the unchecked conformance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Sources/Features/Custom App Icon/DockTilePlugin.swift, line 143:
<comment>Avoid declaring `NSDockTile` as `@unchecked Sendable`. This masks thread-safety issues for a main-thread-only AppKit type; instead keep dock tile updates on the main thread and remove the unchecked conformance.</comment>
<file context>
@@ -0,0 +1,143 @@
+
+// This is required because of the DispatchQueue call above. This doesn't
+// feel right but I don't know a better way to solve this.
+extension NSDockTile: @unchecked @retroactive Sendable {}
</file context>
|
|
||
| case .center: | ||
| return .init(x: round(screen.visibleFrame.origin.x + (screen.visibleFrame.width - window.frame.width) / 2), y: screen.visibleFrame.height - window.frame.width) | ||
| return .init(x: round(screen.visibleFrame.origin.x + (screen.visibleFrame.width - window.frame.width) / 2), y: screen.visibleFrame.height - window.frame.width) |
There was a problem hiding this comment.
P2: Center initial Y origin uses window.frame.width and omits visibleFrame.origin.y, causing incorrect vertical placement during center transitions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift, line 89:
<comment>Center initial Y origin uses `window.frame.width` and omits `visibleFrame.origin.y`, causing incorrect vertical placement during center transitions.</comment>
<file context>
@@ -86,13 +86,13 @@ enum QuickTerminalPosition : String {
case .center:
- return .init(x: round(screen.visibleFrame.origin.x + (screen.visibleFrame.width - window.frame.width) / 2), y: screen.visibleFrame.height - window.frame.width)
+ return .init(x: round(screen.visibleFrame.origin.x + (screen.visibleFrame.width - window.frame.width) / 2), y: screen.visibleFrame.height - window.frame.width)
}
}
</file context>
Suggested change
| return .init(x: round(screen.visibleFrame.origin.x + (screen.visibleFrame.width - window.frame.width) / 2), y: screen.visibleFrame.height - window.frame.width) | |
| return .init(x: round(screen.visibleFrame.origin.x + (screen.visibleFrame.width - window.frame.width) / 2), y: round(screen.visibleFrame.origin.y + (screen.visibleFrame.height - window.frame.height) / 2)) |
|
|
||
| // Fall back to wrapping the created controller if AppKit tab-group | ||
| // bookkeeping has not fully refreshed in the current run loop. | ||
| let fallbackWindow = ScriptWindow(primaryController: createdController) |
There was a problem hiding this comment.
P2: The new tab fallback can return a tab object anchored to a transient window ID. If tab-group state hasn’t settled yet, later AppleScript resolution of that returned tab may fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Sources/Features/AppleScript/AppDelegate+AppleScript.swift, line 291:
<comment>The `new tab` fallback can return a tab object anchored to a transient window ID. If tab-group state hasn’t settled yet, later AppleScript resolution of that returned tab may fail.</comment>
<file context>
@@ -0,0 +1,351 @@
+
+ // Fall back to wrapping the created controller if AppKit tab-group
+ // bookkeeping has not fully refreshed in the current run loop.
+ let fallbackWindow = ScriptWindow(primaryController: createdController)
+ return ScriptTab(window: fallbackWindow, controller: createdController)
+ }
</file context>
| if (ghostty.config.bellFeatures.contains(.attention)) { | ||
| if ghostty.config.bellFeatures.contains(.audio) { | ||
| if let configPath = ghostty.config.bellAudioPath, | ||
| let sound = NSSound(contentsOfFile: configPath.path, byReference: false) { |
There was a problem hiding this comment.
P2: The local NSSound instance will be deallocated by ARC before the asynchronous playback completes, causing the custom bell audio to cut off or fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At macos/Sources/App/macOS/AppDelegate.swift, line 782:
<comment>The local `NSSound` instance will be deallocated by ARC before the asynchronous playback completes, causing the custom bell audio to cut off or fail.</comment>
<file context>
@@ -775,25 +773,35 @@ class AppDelegate: NSObject,
- if (ghostty.config.bellFeatures.contains(.attention)) {
+ if ghostty.config.bellFeatures.contains(.audio) {
+ if let configPath = ghostty.config.bellAudioPath,
+ let sound = NSSound(contentsOfFile: configPath.path, byReference: false) {
+ sound.volume = ghostty.config.bellAudioVolume
+ sound.play()
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by cubic
Keeps embedded terminal surfaces aligned with moving pane strips during horizontal animations and pans, removing lag and overlap. Addresses Linear 1221 (paper window manager layout) and improves
cmuxpaper-canvas motion harness compatibility.cmuxmotion harness.Written for commit 937af72. Summary will update on new commits.