On Jun 14, 2015, at 1:11 PM, Peter Maydell wrote: > On 18 May 2015 at 17:23, Programmingkid <programmingk...@gmail.com> wrote: >> Adds all removable devices to the Machine menu as a Change and Eject menu >> item pair. ide-cd0 would have a "Change ide-cd0..." and "Eject ide-cd0" >> menu items. >> >> Signed-off-by: John Arbuckle <programmingk...@gmail.com> > > I'm afraid this one still needs a bit more work on the > detail, though I think it's OK in general approach. > >> --- >> Replace NSRunAlertPanel() with QEMU_Alert(). >> Free currentDevice variable after finished using it. >> Add "Removable Media" text to the top of devices menu items for easier >> identification. >> Replace depreciated code. >> >> ui/cocoa.m | 140 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 140 insertions(+), 0 deletions(-) >> >> diff --git a/ui/cocoa.m b/ui/cocoa.m >> index 559058b..3acde50 100644 >> --- a/ui/cocoa.m >> +++ b/ui/cocoa.m >> @@ -30,6 +30,7 @@ >> #include "ui/input.h" >> #include "sysemu/sysemu.h" >> #include "qmp-commands.h" >> +#include "sysemu/blockdev.h" >> >> >> >> #ifndef MAC_OS_X_VERSION_10_5 >> #define MAC_OS_X_VERSION_10_5 1050 >> @@ -242,7 +243,27 @@ static int cocoa_keycode_to_qemu(int keycode) >> return keymap[keycode]; >> } >> >> >> >> +/* Displays an alert dialog box with the specified message */ >> +static void QEMU_Alert(NSString *message) >> +{ >> + NSAlert *alert; >> + alert = [NSAlert alertWithMessageText:message >> + defaultButton:@"OK" >> + alternateButton:nil >> + otherButton:nil >> + informativeTextWithFormat:@""]; > > This gives a deprecation warning on 10.10: > > /Users/pm215/src/qemu/ui/cocoa.m:250:22: warning: > > 'alertWithMessageText:defaultButton:alternateButton:otherButton:informativeTextWithFormat:' > is deprecated: first > deprecated in OS X 10.10 - Use -init instead [-Wdeprecated-declarations] > alert = [NSAlert alertWithMessageText:message > ^ > /System/Library/Frameworks/AppKit.framework/Headers/NSAlert.h:70:1: note: > > 'alertWithMessageText:defaultButton:alternateButton:otherButton:informativeTextWithFormat:' > has been explicitly marked > deprecated here > + (NSAlert *)alertWithMessageText:(NSString *)message > defaultButton:(NSString *)defaultButton alternateButton:(NSStri... > ^
Apple is becoming depreciation crazy. Ok, will fix this problem. >> + [alert runModal]; >> +} >> >> > >> +/* Displays a dialog box asking the user to select an image file to load. >> + * Uses sender's tag value to figure out which drive to use. >> + */ >> +- (void)changeDeviceMedia:(id)sender >> +{ >> + /* Find the drive name */ >> + NSString * drive; >> + drive = [sender representedObject]; >> + if(drive == nil) { >> + NSBeep(); >> + QEMU_Alert(@"Could not find drive!"); >> + return; >> + } >> + >> + /* Display the file open dialog */ >> + NSOpenPanel * openPanel; >> + openPanel = [NSOpenPanel openPanel]; >> + [openPanel setCanChooseFiles: YES]; >> + [openPanel setAllowsMultipleSelection: NO]; >> + [openPanel setAllowedFileTypes: [NSArray arrayWithObjects: @"iso", >> @"cdr", @"img", @"dmg", nil]]; > > This list is too short (no qcow2, among other things) and doesn't match > the one we use for the initial image. Ideally we should use the same > code for "let user pick initial disk image" and "let user pick new > image for this drive". Is this the complete list you want: iso, cdr, img, dmg, cow, qcow, qcow2, vmdk, cloop, vpc, vdi > >> + if([openPanel runModal] == NSFileHandlingPanelOKButton) { >> + NSString * file = [[openPanel filenames] objectAtIndex: 0]; > > Another deprecation warning: > /Users/pm215/src/qemu/ui/cocoa.m:1112:39: warning: 'filenames' is > deprecated: first deprecated in OS X 10.6 > [-Wdeprecated-declarations] > NSString * file = [[openPanel filenames] objectAtIndex: 0]; > ^ > /System/Library/Frameworks/AppKit.framework/Headers/NSOpenPanel.h:51:1: > note: 'filenames' has been explicitly marked > deprecated here > - (NSArray *)filenames NS_DEPRECATED_MAC(10_0, 10_6); > ^ > Why Apple why? Ok, I will see what I can do for this issue. > > >> + Error *err = NULL; >> + qmp_change_blockdev([drive cStringUsingEncoding: >> NSASCIIStringEncoding], [file cStringUsingEncoding: NSASCIIStringEncoding], >> "raw", &err); >> + handleAnyDeviceErrors(err); >> + } >> +} >> + >> @end >> >> >> >> >> >> @@ -1260,6 +1329,71 @@ static void add_console_menu_entries(void) >> } >> } >> >> >> >> +/* Make menu items for all removable devices. >> + * Each device is given an 'Eject' and 'Change' menu item. >> + */ >> +static void addRemovableDevicesMenuItems() >> +{ >> + NSMenu *menu; >> + NSMenuItem *menuItem; >> + BlockInfoList *currentDevice; >> + NSString *deviceName; >> + >> + currentDevice = qmp_query_block(NULL); >> + if(currentDevice == NULL) { >> + NSBeep(); >> + QEMU_Alert(@"Failed to query for block devices!"); >> + return; >> + } >> + >> + menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu]; >> + >> + // Add a separator between related groups of menu items >> + [menu addItem:[NSMenuItem separatorItem]]; >> + >> + // Set the attributes to the "Removable Media" menu item >> + NSString *titleString = @"Removable Media"; >> + NSMutableAttributedString *attString=[[NSMutableAttributedString alloc] >> initWithString:titleString]; >> + NSColor *newColor = [NSColor blackColor]; >> + NSFontManager *fontManager = [NSFontManager sharedFontManager]; >> + NSFont *font = [fontManager fontWithFamily:@"Helvetica" >> + >> traits:NSBoldFontMask|NSItalicFontMask >> + weight:0 >> + size:14]; >> + [attString addAttribute:NSFontAttributeName value:font >> range:NSMakeRange(0, [titleString length])]; >> + [attString addAttribute:NSForegroundColorAttributeName value:newColor >> range:NSMakeRange(0, [titleString length])]; >> + [attString addAttribute:NSUnderlineStyleAttributeName value:[NSNumber >> numberWithInt: 1] range:NSMakeRange(0, [titleString length])]; > > This italicised line looks really odd in the menu -- is it really > the way apple's UI guidelines recommend doing this sort of thing? > (It seems like quite a lot of effort to have to go to.) It required a huge amount of effort on my part. But it did look really good, so I really hope you let me keep it :) > >> + >> + // Add the "Removable Media" menu item >> + menuItem = [NSMenuItem new]; >> + [menuItem setAttributedTitle: attString]; >> + [menuItem setEnabled: NO]; >> + [menu addItem: menuItem]; >> + >> + /* Loop thru all the block devices in the emulator */ >> + while (currentDevice) { >> + deviceName = [[NSString stringWithFormat: @"%s", >> currentDevice->value->device] retain]; >> + >> + if(currentDevice->value->removable) { >> + menuItem = [[NSMenuItem alloc] initWithTitle: [NSString >> stringWithFormat: @"Change %s...", currentDevice->value->device] >> + action: >> @selector(changeDeviceMedia:) >> + keyEquivalent: @""]; >> + [menu addItem: menuItem]; >> + [menuItem setRepresentedObject: deviceName]; >> + [menuItem autorelease]; >> + >> + menuItem = [[NSMenuItem alloc] initWithTitle: [NSString >> stringWithFormat: @"Eject %s", currentDevice->value->device] >> + action: >> @selector(ejectDeviceMedia:) >> + keyEquivalent: @""]; >> + [menu addItem: menuItem]; >> + [menuItem setRepresentedObject: deviceName]; >> + [menuItem autorelease]; >> + } >> + currentDevice = currentDevice->next; >> + } >> + free(currentDevice); > > currentDevice will usually be NULL here because you've used > it as your iterator variable. In any case this > isn't the right way to free the pointer you get back from > qmp_query_block() -- you need qapi_free_BlockInfoList(). qapi_free_BlockInfoList(currentDevice); This is what you want? > >> +} >> + >> void cocoa_display_init(DisplayState *ds, int full_screen) >> { >> COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n"); >> @@ -1283,4 +1417,10 @@ void cocoa_display_init(DisplayState *ds, int >> full_screen) >> * menu entries for them. >> */ >> add_console_menu_entries(); >> + >> + /* Give all removable devices a menu item. >> + * Has to be called after QEMU has started to >> + * find out what removable devices it has. >> + */ >> + addRemovableDevicesMenuItems(); >> } >> -- >> 1.7.5.4 > > thanks > -- PMM Thank you for taking the time to review my patches. I will implement your changes as soon as I receive your reply.