Refactor notification registration for MCPMaxUINotification#66
Refactor notification registration for MCPMaxUINotification#66std-microblock wants to merge 1 commit intofosdickio:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors how the Binary Ninja UI notification handler (_MCPMaxUINotification) is registered, likely to improve Windows stability by keeping a persistent reference and adjusting the registration flow (fixes #34).
Changes:
- Adds an
__init__to_MCPMaxUINotificationthat registers the instance withUIContext. - Stores the notification instance in a module-level variable instead of constructing it only as an argument to
registerNotification.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def __init__(self): | ||
| super().__init__() | ||
| ui.UIContext.registerNotification(self) | ||
|
|
There was a problem hiding this comment.
The blank line after ui.UIContext.registerNotification(self) contains trailing whitespace. Please remove the extra spaces to avoid whitespace-only diffs and potential lint/style issues.
| def __init__(self): | ||
| super().__init__() | ||
| ui.UIContext.registerNotification(self) |
There was a problem hiding this comment.
_MCPMaxUINotification.__init__ performs global registration as a side effect. Consider keeping registration in the outer block where the instance is created so the lifecycle is more explicit and it’s harder to accidentally register multiple instances in the future.
| pass | ||
|
|
||
| ui.UIContext.registerNotification(_MCPMaxUINotification()) | ||
| notification = _MCPMaxUINotification() |
There was a problem hiding this comment.
The new module-level notification variable is only used for its side effects / lifetime management, but the name is very generic and could be mistaken for dead code. Consider renaming it to something explicit like _ui_notification (and optionally add a short comment that it must be kept referenced to prevent GC).
| notification = _MCPMaxUINotification() | |
| # Keep a module-level reference so the UI notification is not garbage-collected | |
| _ui_notification = _MCPMaxUINotification() |
fix #34