Closesf7a432a#r61013322. Would be nice if we could run with -Werror instead, which would have caught this, but I assume@dopplershift had a good reason to only use -Werror=unguarded-availability in#17956?

It's also not completely clear to me why the only call to closeButtonPressed (in windowShouldClose) is protected by respondsToSelector, and whether it could just be inlined there instead, using something like

diff --git i/src/_macosx.m w/src/_macosx.mindex 64875fe7f1..cf84ef02fa 100755--- i/src/_macosx.m+++ w/src/_macosx.m@@ -1519,12 +1519,8 @@ - (BOOL)windowShouldClose:(NSNotification*)notification                                            data1: 0                                            data2: 0];     [NSApp postEvent: event atStart: true];-    if ([window respondsToSelector: @selector(closeButtonPressed)]) {-        BOOL closed = [((Window*) window) closeButtonPressed];-        /* If closed, the window has already been closed via the manager. */-        if (closed) { return NO; }-    }-    return YES;+    gil_call_method(manager, "close");+    return NO;  // the window has already been closed via the manager. }  - (void)mouseEntered:(NSEvent *)event

Still, this patch should fix the main problem at hand...

PR Summary

PR Checklist

Tests and Styling

Has pytest style unit tests (andpytest passes).
IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

Fix missing return value in closeButtonPressed.

57e320e

Accidentally removed inf7a432a.

anntzer added the GUI: MacOSX label

Nov 29, 2021

anntzer referenced this pull request

Nov 29, 2021

Factor out some macosx gil handling for py-method calls from callbacks.

f7a432a

Also, explicitly checking for whether zoombutton/panbutton are non-nullis unnecessary, as objc explicitly defines sending a message to null asa noop.  Also, they should never be nil anyways...

greglucas approved these changes

Nov 29, 2021

View reviewed changes

Copy link

Contributor

greglucas left a comment

Choose a reason for hiding this comment

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

This update looks good.

For the other updates:

-Werror seems to compile fine for me locally without any warnings, so I agree this seems sensible to add in.
The manager isn't available on the view there, so I think we'd have to store that somewhere on the window and then call super to get to that method. However, I'm a bit confused why we need that chunk of code at all... I think that even callingwindow.close() programatically would have closed the window by this point, so we shouldn't have to worry about that case?
Dropping that code works for me interactively with multiple windows as expected.

diff --git a/src/_macosx.m b/src/_macosx.mindex 64875fe7f1..5e3bd4fc96 100755--- a/src/_macosx.m+++ b/src/_macosx.m@@ -226,7 +226,6 @@ - (void)windowDidResize:(NSNotification*)notification; - (View*)initWithFrame:(NSRect)rect; - (void)setCanvas: (PyObject*)newCanvas; - (void)windowWillClose:(NSNotification*)notification;-- (BOOL)windowShouldClose:(NSNotification*)notification; - (BOOL)isFlipped; - (void)mouseEntered:(NSEvent*)event; - (void)mouseExited:(NSEvent*)event;@@ -1291,7 +1290,11 @@ - (NSRect)constrainFrameRect:(NSRect)rect toScreen:(NSScreen*)screen     return suggested; }-- (BOOL)closeButtonPressed { gil_call_method(manager, "close"); }+- (BOOL)closeButtonPressed+{+    gil_call_method(manager, "close");+    return YES;+} - (void)close {@@ -1506,27 +1509,6 @@ - (void)windowWillClose:(NSNotification*)notification     PyGILState_Release(gstate); }-- (BOOL)windowShouldClose:(NSNotification*)notification-{-    NSWindow* window = [self window];-    NSEvent* event = [NSEvent otherEventWithType: NSEventTypeApplicationDefined-                                        location: NSZeroPoint-                                   modifierFlags: 0-                                       timestamp: 0.0-                                    windowNumber: 0-                                         context: nil-                                          subtype: WINDOW_CLOSING-                                           data1: 0-                                           data2: 0];-    [NSApp postEvent: event atStart: true];-    if ([window respondsToSelector: @selector(closeButtonPressed)]) {-        BOOL closed = [((Window*) window) closeButtonPressed];-        /* If closed, the window has already been closed via the manager. */-        if (closed) { return NO; }-    }-    return YES;-}- - (void)mouseEntered:(NSEvent *)event {     PyGILState_STATE gstate;