qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: Adds device menu items to Machin


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: Adds device menu items to Machine menu
Date: Sun, 14 Jun 2015 18:11:09 +0100

On 18 May 2015 at 17:23, Programmingkid <address@hidden> 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 <address@hidden>

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...
^

> +    [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".

> +    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);
^



> +        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.)

> +
> +    // 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().

> +}
> +
>  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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]