Re: [Qemu-devel] [patch v5] ui/cocoa.m: Adds console items to the View menu

[Programmingkid]

On May 16, 2015, at 5:45 PM, Peter Maydell wrote:

> On 11 May 2015 at 16:18, Programmingkid <address@hidden> wrote:
>> Adds any console that is available to the current emulator as a menu item 
>> under the View menu.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
> 
> Thanks; I think this version looks pretty good. A minor
> improvement:
> 
>> +static void create_view_menu()
>> +{
>> +    NSMenu * menu;
>> +    NSMenuItem * menuItem;
>> +    int index = 0;
>> +
>> +    menu = [[NSMenu alloc] initWithTitle:@"View"];
>> +    [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" 
>> action:@selector(toggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // 
>> Fullscreen
>> +    [menu addItem:[NSMenuItem separatorItem]]; //Separator
>> +
>> +    // Give each console its own menu item in the View menu
>> +    while(qemu_console_lookup_by_index(index) != NULL) {
>> +        menuItem = [[[NSMenuItem alloc] initWithTitle: 
>> getConsoleName(qemu_console_lookup_by_index(index))
>> +                                               action: 
>> @selector(displayConsole:) keyEquivalent: @""] autorelease];
>> +        [menuItem setTag: index];
>> +        [menu addItem: menuItem];
>> +        index++;
>> +    }
>> +
>> +    menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil 
>> keyEquivalent:@""] autorelease];
>> +    [menuItem setSubmenu:menu];
>> +    [[NSApp mainMenu] insertItem: menuItem atIndex: 1]; // insert View menu 
>> after Application menu
>> +}
> 
> Rather than creating the whole menu here, and then having to
> use awkward hard-coded indexes to decide where to put it in
> the menubar, I think it would be nicer to leave the creation
> of the menu (and its fixed entries) where it is and just do
> the addition of new items here. You can get the View menu with:
> 
>    menu = [[[NSApp mainMenu] itemWithTitle:@"View"] submenu];
> 
> and then just [menu addItem ... ] the new items to it.
> 
> I also had to tweak the patch a bit to get it to apply on top
> of the fullscreen patches, so I went ahead and made that
> change, and applied the resulting patch to my cocoa.next
> branch in
> https://git.linaro.org/people/peter.maydell/qemu-arm.git
> 
> Let me know if you'd rather do something else.
> 
> thanks
> -- PMM

What you did looks good. 

You know how there's a "Zoom To Fit" menu item. What if we added a mutually 
exclusive menu item called "Resize To Fit"? This menu item would instead adjust 
the host screen's resolution to match that of the guest screen's resolution. It 
can be implemented in its own patch if you like the idea.