Skip to content

macOS: avoid holding SurfaceView when sending notifications - #13787

Closed
bo2themax wants to merge 1 commit into
ghostty-org:mainfrom
bo2themax:push-74857782
Closed

macOS: avoid holding SurfaceView when sending notifications#13787
bo2themax wants to merge 1 commit into
ghostty-org:mainfrom
bo2themax:push-74857782

Conversation

@bo2themax

Copy link
Copy Markdown
Member

We shouldn't hold a closing surface view when sending notifications. This happens rarely, but it's the right thing to do.

AI Disclosure

Found by Claude when judging other branches, I applied the changes myself.

@bo2themax
bo2themax requested a review from a team as a code owner August 13, 2026 16:33
@ghostty-bot ghostty-bot Bot added the os/macos label Aug 13, 2026
// We need to keep track of this notification so we can remove it
// under certain circumstances
notificationIdentifiers.insert(uuid)
self?.notificationIdentifiers.insert(uuid)

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.

Maybe guard let self ... here to simplify the rest of this block's calling patterns? Everything afterward is dependent on self being available.

@bo2themax bo2themax Aug 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We still need to remove the notification using uuid 3s later. I would imagine at least copy the check twice馃檲

@jparise jparise Aug 13, 2026

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.

If that's the case, then isn't the if self?.focused == true condition going to fail and prevent the UUID-based removal from running anyway?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh yeah, you are right

@bo2themax bo2themax closed this Aug 14, 2026
mitchellh added a commit that referenced this pull request Aug 14, 2026
We shouldn't hold a closing surface view when sending notifications and
waiting to dismiss that notification. This happens rarely, but it's the
right thing to do.

### AI Disclosure
Found by Claude when judging other branches, I applied the changes
myself.

> Forgot that after force pushing, you can't reopen #13787 馃, linking it
here for the review history.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants