diff mbox

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

Message ID 71B9096B-B9D6-4929-9212-0D55A5C35CC8@gmail.com
State New
Headers show

Commit Message

Programmingkid Sept. 18, 2015, 11: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
wants to quit QEMU.

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

---
 ui/cocoa.m |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

Comments

Peter Maydell Sept. 20, 2015, 12:21 p.m. UTC | #1
On 19 September 2015 at 00: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
> wants to quit QEMU.

"if they want to" -- our users are not exclusively male :-)

> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>
> ---
>  ui/cocoa.m |   22 +++++++++++++++++++++-
>  1 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..ff1a72b 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -829,6 +829,7 @@ QemuCocoaView *cocoaView;
>  - (void)powerDownQEMU:(id)sender;
>  - (void)ejectDeviceMedia:(id)sender;
>  - (void)changeDeviceMedia:(id)sender;
> +- (void)verifyQuit;
>  @end
>
>
>
>  @implementation QemuCocoaAppController
> @@ -862,6 +863,7 @@ QemuCocoaView *cocoaView;
>  #endif
>          [normalWindow makeKeyAndOrderFront:self];
>          [normalWindow center];
> +        [normalWindow setDelegate: self];
>          stretch_video = false;
>
>
>
>          /* Used for displaying pause on the screen */
> @@ -933,6 +935,13 @@ QemuCocoaView *cocoaView;
>      return YES;
>  }
>
>
>
> +/* Called when the user clicks on a window's close button */
> +- (BOOL)windowShouldClose:(id)sender
> +{
> +    [self verifyQuit];
> +    return NO;
> +}
> +
>  - (void)startEmulationWithArgc:(int)argc argv:(char**)argv
>  {
>      COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
> @@ -1125,6 +1134,16 @@ QemuCocoaView *cocoaView;
>      }
>  }
>
>
>
> +/* Verifies if the user really wants to quit */
> +- (void)verifyQuit
> +{
> +    NSInteger response;
> +    response = NSRunAlertPanel(@"Quit?", @"Are you sure you want to quit?",
> +                                                       @"Cancel", @"Quit",
> nil);
> +    if(response == NSAlertAlternateReturn)
> +        exit(EXIT_SUCCESS);

Missing braces. Checkpatch will warn you about this kind of thing.

Calling exit() here doesn't really seem right to me -- I think we
should return the "OK to terminate app/close this window"
response to the OSX framework and let it cause us to exit in
the normal way.

> +}
> +
>  @end
>
>
>
>
>
> @@ -1178,7 +1197,8 @@ int main (int argc, const char * argv[]) {
>      [menuItem
> setKeyEquivalentModifierMask:(NSAlternateKeyMask|NSCommandKeyMask)];
>      [menu addItemWithTitle:@"Show All"
> action:@selector(unhideAllApplications:) keyEquivalent:@""]; // Show All
>      [menu addItem:[NSMenuItem separatorItem]]; //Separator
> -    [menu addItemWithTitle:@"Quit QEMU" action:@selector(terminate:)
> keyEquivalent:@"q"];
> +    [menu addItemWithTitle:@"Quit QEMU" action:@selector(verifyQuit)

Should we be doing this by implementing
NSApplication:applicationShouldTerminate?
That seems to be where the OSX framework expects us to handle
"do you really want to do this" as part of the standard terminate:
flow.

> +
> keyEquivalent:@"q"];
>      menuItem = [[NSMenuItem alloc] initWithTitle:@"Apple" action:nil
> keyEquivalent:@""];
>      [menuItem setSubmenu:menu];
>      [[NSApp mainMenu] addItem:menuItem];

thanks
-- PMM
Programmingkid Sept. 20, 2015, 4:19 p.m. UTC | #2
On Sep 20, 2015, at 8:21 AM, Peter Maydell wrote:

> On 19 September 2015 at 00: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
>> wants to quit QEMU.
> 
> "if they want to" -- our users are not exclusively male :-)

I'm thinking "if he or she wants to" would be more grammatically correct. 

> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> ---
>> ui/cocoa.m |   22 +++++++++++++++++++++-
>> 1 files changed, 21 insertions(+), 1 deletions(-)
>> 
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 334e6f6..ff1a72b 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -829,6 +829,7 @@ QemuCocoaView *cocoaView;
>> - (void)powerDownQEMU:(id)sender;
>> - (void)ejectDeviceMedia:(id)sender;
>> - (void)changeDeviceMedia:(id)sender;
>> +- (void)verifyQuit;
>> @end
>> 
>> 
>> 
>> @implementation QemuCocoaAppController
>> @@ -862,6 +863,7 @@ QemuCocoaView *cocoaView;
>> #endif
>>         [normalWindow makeKeyAndOrderFront:self];
>>         [normalWindow center];
>> +        [normalWindow setDelegate: self];
>>         stretch_video = false;
>> 
>> 
>> 
>>         /* Used for displaying pause on the screen */
>> @@ -933,6 +935,13 @@ QemuCocoaView *cocoaView;
>>     return YES;
>> }
>> 
>> 
>> 
>> +/* Called when the user clicks on a window's close button */
>> +- (BOOL)windowShouldClose:(id)sender
>> +{
>> +    [self verifyQuit];
>> +    return NO;
>> +}
>> +
>> - (void)startEmulationWithArgc:(int)argc argv:(char**)argv
>> {
>>     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
>> @@ -1125,6 +1134,16 @@ QemuCocoaView *cocoaView;
>>     }
>> }
>> 
>> 
>> 
>> +/* Verifies if the user really wants to quit */
>> +- (void)verifyQuit
>> +{
>> +    NSInteger response;
>> +    response = NSRunAlertPanel(@"Quit?", @"Are you sure you want to quit?",
>> +                                                       @"Cancel", @"Quit",
>> nil);
>> +    if(response == NSAlertAlternateReturn)
>> +        exit(EXIT_SUCCESS);
> 
> Missing braces. Checkpatch will warn you about this kind of thing.
Surprisingly it didn't work. I have noticed that checkpatch also doesn't detect lines over
80 characters in length. I did use checkpatch on this patch. It said there were no
errors or warnings. Maybe it doesn't know objective-c very well?

> Calling exit() here doesn't really seem right to me -- I think we
> should return the "OK to terminate app/close this window"
> response to the OSX framework and let it cause us to exit in
> the normal way.

Ok.

> 
>> +}
>> +
>> @end
>> 
>> 
>> 
>> 
>> 
>> @@ -1178,7 +1197,8 @@ int main (int argc, const char * argv[]) {
>>     [menuItem
>> setKeyEquivalentModifierMask:(NSAlternateKeyMask|NSCommandKeyMask)];
>>     [menu addItemWithTitle:@"Show All"
>> action:@selector(unhideAllApplications:) keyEquivalent:@""]; // Show All
>>     [menu addItem:[NSMenuItem separatorItem]]; //Separator
>> -    [menu addItemWithTitle:@"Quit QEMU" action:@selector(terminate:)
>> keyEquivalent:@"q"];
>> +    [menu addItemWithTitle:@"Quit QEMU" action:@selector(verifyQuit)
> 
> Should we be doing this by implementing
> NSApplication:applicationShouldTerminate?
> That seems to be where the OSX framework expects us to handle
> "do you really want to do this" as part of the standard terminate:
> flow.

Ok. I can do that if you wish. 

> 
>> +
>> keyEquivalent:@"q"];
>>     menuItem = [[NSMenuItem alloc] initWithTitle:@"Apple" action:nil
>> keyEquivalent:@""];
>>     [menuItem setSubmenu:menu];
>>     [[NSApp mainMenu] addItem:menuItem];
> 
> thanks
> -- PMM
Peter Maydell Sept. 21, 2015, 2:32 a.m. UTC | #3
On 20 September 2015 at 17:19, Programmingkid <programmingkidx@gmail.com> wrote:
>
> On Sep 20, 2015, at 8:21 AM, Peter Maydell wrote:
>
>> On 19 September 2015 at 00: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
>>> wants to quit QEMU.
>>
>> "if they want to" -- our users are not exclusively male :-)
>
> I'm thinking "if he or she wants to" would be more grammatically correct.

Singular 'they' is totally fine and has been for centuries, and it's less
clunky than 'he or she'. But you can use the latter if you prefer it.

thanks
-- PMM
diff mbox

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 334e6f6..ff1a72b 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -829,6 +829,7 @@  QemuCocoaView *cocoaView;
 - (void)powerDownQEMU:(id)sender;
 - (void)ejectDeviceMedia:(id)sender;
 - (void)changeDeviceMedia:(id)sender;
+- (void)verifyQuit;
 @end
 
 @implementation QemuCocoaAppController
@@ -862,6 +863,7 @@  QemuCocoaView *cocoaView;
 #endif
         [normalWindow makeKeyAndOrderFront:self];
         [normalWindow center];
+        [normalWindow setDelegate: self];
         stretch_video = false;
 
         /* Used for displaying pause on the screen */
@@ -933,6 +935,13 @@  QemuCocoaView *cocoaView;
     return YES;
 }
 
+/* Called when the user clicks on a window's close button */
+- (BOOL)windowShouldClose:(id)sender
+{
+    [self verifyQuit];
+    return NO;
+}
+
 - (void)startEmulationWithArgc:(int)argc argv:(char**)argv
 {
     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
@@ -1125,6 +1134,16 @@  QemuCocoaView *cocoaView;
     }
 }
 
+/* Verifies if the user really wants to quit */
+- (void)verifyQuit
+{
+    NSInteger response;
+    response = NSRunAlertPanel(@"Quit?", @"Are you sure you want to quit?",
+                                                       @"Cancel", @"Quit", nil);
+    if(response == NSAlertAlternateReturn)
+        exit(EXIT_SUCCESS);
+}
+
 @end
 
 
@@ -1178,7 +1197,8 @@  int main (int argc, const char * argv[]) {
     [menuItem setKeyEquivalentModifierMask:(NSAlternateKeyMask|NSCommandKeyMask)];
     [menu addItemWithTitle:@"Show All" action:@selector(unhideAllApplications:) keyEquivalent:@""]; // Show All
     [menu addItem:[NSMenuItem separatorItem]]; //Separator
-    [menu addItemWithTitle:@"Quit QEMU" action:@selector(terminate:) keyEquivalent:@"q"];
+    [menu addItemWithTitle:@"Quit QEMU" action:@selector(verifyQuit)
+                                                            keyEquivalent:@"q"];
     menuItem = [[NSMenuItem alloc] initWithTitle:@"Apple" action:nil keyEquivalent:@""];
     [menuItem setSubmenu:menu];
     [[NSApp mainMenu] addItem:menuItem];