diff mbox

[v2] ui/cocoa.m: verify with user before quitting QEMU

Message ID 44201F82-5BFF-4346-A1F7-3798EBDC4180@gmail.com
State New
Headers show

Commit Message

Programmingkid Sept. 20, 2015, 7:20 p.m. UTC
This patch prevents the user from accidentally quitting QEMU by pushing 
Command-Q or by pushing the close button on the main window. When the user does
one of these two things, a dialog box appears verifying with the user if he or
she wants to quit QEMU.

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

---
 ui/cocoa.m |   35 +++++++++++++++++++++++++++++++++--
 1 files changed, 33 insertions(+), 2 deletions(-)

Comments

Peter Maydell Sept. 23, 2015, 5:55 p.m. UTC | #1
On 20 September 2015 at 12:20, Programmingkid <programmingkidx@gmail.com> wrote:
> This patch prevents the user from accidentally quitting QEMU by pushing
> Command-Q or by pushing the close button on the main window. When the user does
> one of these two things, a dialog box appears verifying with the user if he or
> she wants to quit QEMU.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

Thanks for the respin. I generally like this patch; I have a few minor
tweaks to suggest below, and there are a couple of deprecation
warnings if you build on OSX 10.10, but otherwise it looks good.

>
> ---
>  ui/cocoa.m |   35 +++++++++++++++++++++++++++++++++--
>  1 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..3c5172c 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -807,10 +807,11 @@ QemuCocoaView *cocoaView;
>      QemuCocoaAppController
>   ------------------------------------------------------
>  */
> -@interface QemuCocoaAppController : NSObject
> +@interface QemuCocoaAppController : NSObject <NSWindowDelegate
>  #if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6)
> -                                             <NSApplicationDelegate>
> +                                             , NSApplicationDelegate
>  #endif
> +>

The docs say that NSWindowDelegate is 10.6 and later, so
I think you need the whole <NSWindowDelegate, NSApplicationDelegate>
inside the #if.

(10.6 is when the formal protocol interface stuff came in;
prior to that the delegation stuff was what the docs call an
"informal protocol", with no type checking. So I think the
actual implementation of the NSWindowDelegate functions in
the class will work ok on 10.5.)

>  {
>  }
>  - (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
> @@ -829,6 +830,7 @@ QemuCocoaView *cocoaView;
>  - (void)powerDownQEMU:(id)sender;
>  - (void)ejectDeviceMedia:(id)sender;
>  - (void)changeDeviceMedia:(id)sender;
> +- (BOOL)verifyQuit;
>  @end
>
>  @implementation QemuCocoaAppController
> @@ -862,6 +864,7 @@ QemuCocoaView *cocoaView;
>  #endif
>          [normalWindow makeKeyAndOrderFront:self];
>          [normalWindow center];
> +        [normalWindow setDelegate: self];
>          stretch_video = false;
>
>          /* Used for displaying pause on the screen */
> @@ -933,6 +936,21 @@ QemuCocoaView *cocoaView;
>      return YES;
>  }
>
> +- (NSApplicationTerminateReply)applicationShouldTerminate:
> +                                                         (NSApplication *)sender
> +{
> +    COCOA_DEBUG("QemuCocoaAppController: applicationShouldTerminate\n");
> +    return [self verifyQuit];
> +}
> +
> +/* Called when the user clicks on a window's close button */
> +- (BOOL)windowShouldClose:(id)sender
> +{
> +    COCOA_DEBUG("QemuCocoaAppController: windowShouldClose\n");
> +    [NSApp terminate: sender];

I think it would be more informative here to say:

 /* If the user allows the application to quit then the call to
  * NSApp terminate will never return. If we get here then the user
  * cancelled the quit, so we should return NO to not permit the
  * closing of this window.
  */

> +    return NO; /* The main window should always be available */
> +}
> +
>  - (void)startEmulationWithArgc:(int)argc argv:(char**)argv
>  {
>      COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
> @@ -1125,6 +1143,19 @@ QemuCocoaView *cocoaView;
>      }
>  }
>
> +/* Verifies if the user really wants to quit */
> +- (BOOL)verifyQuit
> +{
> +    NSInteger response;
> +    response = NSRunAlertPanel(@"Quit?", @"Are you sure you want to quit?",
> +                                                       @"Cancel", @"Quit", nil);
> +    if(response == NSAlertAlternateReturn) {
> +        return YES; /* Yes, I want to quit */
> +    } else {
> +        return NO; /* No, I don't want to quit */

Not sure the comments here really add much information.

> +    }
> +}

There are some OSX 10.10 deprecation warnings from this code:

  OBJC  ui/cocoa.o
/Users/pm215/src/qemu/ui/cocoa.m:1150:16: warning: 'NSRunAlertPanel'
is deprecated: first deprecated in OS X 10.10 - Use NSAlert instead
[-Wdeprecated-declarations]
    response = NSRunAlertPanel(@"Quit?", @"Are you sure you want to quit?",
               ^
/System/Library/Frameworks/AppKit.framework/Headers/NSPanel.h:48:25:
note: 'NSRunAlertPanel' has been explicitly marked deprecated here
APPKIT_EXTERN NSInteger NSRunAlertPanel(NSString *title, NSString
*msgFormat, NSString *defaultButton, NSString *alternateButton,
NSString *otherButton, ...) NS_FORMAT_FUNCTION(2,6) ...
                        ^
/Users/pm215/src/qemu/ui/cocoa.m:1152:20: warning:
'NSAlertAlternateReturn' is deprecated: first deprecated in OS X 10.10
- Use NSAlertFirstButtonReturn, etc instead
      [-Wdeprecated-declarations]
    if(response == NSAlertAlternateReturn) {
                   ^
/System/Library/Frameworks/AppKit.framework/Headers/NSPanel.h:80:5:
note: 'NSAlertAlternateReturn' has been explicitly marked deprecated
here
    NSAlertAlternateReturn NS_ENUM_DEPRECATED_MAC(10_0, 10_10, "Use
NSAlertFirstButtonReturn, etc instead") = 0,
    ^

thanks
-- PMM
diff mbox

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 334e6f6..3c5172c 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -807,10 +807,11 @@  QemuCocoaView *cocoaView;
     QemuCocoaAppController
  ------------------------------------------------------
 */
-@interface QemuCocoaAppController : NSObject
+@interface QemuCocoaAppController : NSObject <NSWindowDelegate
 #if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6)
-                                             <NSApplicationDelegate>
+                                             , NSApplicationDelegate
 #endif
+>
 {
 }
 - (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
@@ -829,6 +830,7 @@  QemuCocoaView *cocoaView;
 - (void)powerDownQEMU:(id)sender;
 - (void)ejectDeviceMedia:(id)sender;
 - (void)changeDeviceMedia:(id)sender;
+- (BOOL)verifyQuit;
 @end
 
 @implementation QemuCocoaAppController
@@ -862,6 +864,7 @@  QemuCocoaView *cocoaView;
 #endif
         [normalWindow makeKeyAndOrderFront:self];
         [normalWindow center];
+        [normalWindow setDelegate: self];
         stretch_video = false;
 
         /* Used for displaying pause on the screen */
@@ -933,6 +936,21 @@  QemuCocoaView *cocoaView;
     return YES;
 }
 
+- (NSApplicationTerminateReply)applicationShouldTerminate:
+                                                         (NSApplication *)sender
+{
+    COCOA_DEBUG("QemuCocoaAppController: applicationShouldTerminate\n");
+    return [self verifyQuit];
+}
+
+/* Called when the user clicks on a window's close button */
+- (BOOL)windowShouldClose:(id)sender
+{
+    COCOA_DEBUG("QemuCocoaAppController: windowShouldClose\n");
+    [NSApp terminate: sender];
+    return NO; /* The main window should always be available */
+}
+
 - (void)startEmulationWithArgc:(int)argc argv:(char**)argv
 {
     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
@@ -1125,6 +1143,19 @@  QemuCocoaView *cocoaView;
     }
 }
 
+/* Verifies if the user really wants to quit */
+- (BOOL)verifyQuit
+{
+    NSInteger response;
+    response = NSRunAlertPanel(@"Quit?", @"Are you sure you want to quit?",
+                                                       @"Cancel", @"Quit", nil);
+    if(response == NSAlertAlternateReturn) {
+        return YES; /* Yes, I want to quit */
+    } else {
+        return NO; /* No, I don't want to quit */
+    }
+}
+
 @end