Re: [Qemu-devel] PATCH] ui/cocoa.m: verify with user before quitting QEMU

[Programmingkid <programmingkidx@gmail.com>]

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