| Message ID | BE18AD69-1793-4132-BD70-1A6883167901@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 11 May 2015 at 01:32, Programmingkid <programmingkidx@gmail.com> wrote: > This patch adds the VGA, Monitor, Serial, and Parallel menu item to the view > menu. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > + // set the console variables for the consoles we have > + while(qemu_console_lookup_by_index(index) != NULL) { > + console_name = > qemu_console_get_label(qemu_console_lookup_by_index(index)); > + if(strstr(console_name, "VGA") != NULL) { > + graphics_console = index; > + } else if (strstr(console_name, "monitor") != NULL) { > + monitor_console = index; > + } else if (strstr(console_name, "serial") != NULL) { > + serial_console = index; > + } else if (strstr(console_name, "parallel") != NULL) { > + parallel_console = index; > + } else { > + printf("Error in initConsoleVariables(): given value %s\n", > console_name); Can't we just create a menu with an item for every console, and set its menu text to the result of qemu_console_get_label() ? I don't see why we need to special case these four and only display those. thanks -- PMM
On May 11, 2015, at 9:20 AM, Peter Maydell wrote: > On 11 May 2015 at 01:32, Programmingkid <programmingkidx@gmail.com> wrote: >> This patch adds the VGA, Monitor, Serial, and Parallel menu item to the view >> menu. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > >> + // set the console variables for the consoles we have >> + while(qemu_console_lookup_by_index(index) != NULL) { >> + console_name = >> qemu_console_get_label(qemu_console_lookup_by_index(index)); >> + if(strstr(console_name, "VGA") != NULL) { >> + graphics_console = index; >> + } else if (strstr(console_name, "monitor") != NULL) { >> + monitor_console = index; >> + } else if (strstr(console_name, "serial") != NULL) { >> + serial_console = index; >> + } else if (strstr(console_name, "parallel") != NULL) { >> + parallel_console = index; >> + } else { >> + printf("Error in initConsoleVariables(): given value %s\n", >> console_name); > > Can't we just create a menu with an item for every console, > and set its menu text to the result of qemu_console_get_label() ? > I don't see why we need to special case these four and only > display those. > > thanks > -- PMM We *could* do it that way, but it wouldn't look pretty. Instead of having these menu items: VGA, Monitor, Serial, Parallel We would have these menu items: VGA compat_monitor0 serial0 parallel0 They look very anti-pretty.
On 11 May 2015 at 15:29, Programmingkid <programmingkidx@gmail.com> wrote: > > On May 11, 2015, at 9:20 AM, Peter Maydell wrote: >> Can't we just create a menu with an item for every console, >> and set its menu text to the result of qemu_console_get_label() ? >> I don't see why we need to special case these four and only >> display those. > We *could* do it that way, but it wouldn't look pretty. Instead of having these menu items: > > VGA, Monitor, Serial, Parallel > > We would have these menu items: > > VGA > compat_monitor0 > serial0 > parallel0 > > They look very anti-pretty. They're the menu entries we have in the GTK UI. If you want pretty names then you should fix this in the common code so that any UI that wants to display pretty names can do that. Having the OSX UI do things differently is not a good idea. -- PMM
On May 11, 2015, at 10:33 AM, Peter Maydell wrote: > On 11 May 2015 at 15:29, Programmingkid <programmingkidx@gmail.com> wrote: >> >> On May 11, 2015, at 9:20 AM, Peter Maydell wrote: >>> Can't we just create a menu with an item for every console, >>> and set its menu text to the result of qemu_console_get_label() ? >>> I don't see why we need to special case these four and only >>> display those. > >> We *could* do it that way, but it wouldn't look pretty. Instead of having these menu items: >> >> VGA, Monitor, Serial, Parallel >> >> We would have these menu items: >> >> VGA >> compat_monitor0 >> serial0 >> parallel0 >> >> They look very anti-pretty. > > They're the menu entries we have in the GTK UI. If you want > pretty names then you should fix this in the common code > so that any UI that wants to display pretty names can do > that. Having the OSX UI do things differently is not a > good idea. I know your big on having the interface be consistent between operating systems, but my experience is Linux users use only Linux software and Mac users use only Mac software. It is possible there is someone out there who uses both the Mac OS and Linux versions of QEMU. I'm just not going to bet on it. I will have a patch for your idea available soon.
On 11 May 2015 at 15:38, Programmingkid <programmingkidx@gmail.com> wrote: > On May 11, 2015, at 10:33 AM, Peter Maydell wrote: >> They're the menu entries we have in the GTK UI. If you want >> pretty names then you should fix this in the common code >> so that any UI that wants to display pretty names can do >> that. Having the OSX UI do things differently is not a >> good idea. > > I know your big on having the interface be consistent between > operating systems, but my experience is Linux users use only > Linux software and Mac users use only Mac software. It is possible > there is someone out there who uses both the Mac OS and Linux > versions of QEMU. I'm just not going to bet on it. The point is not so much to help people who switch between the two UIs, but to keep the code which is OSX specific restricted to really OSX specific things. Code which is not common to all QEMU configurations is harder to test and maintain, especially for OSX where almost nobody in the active dev community runs on it. "Menu entries could be more user friendly" is not at all OSX specific -- if we implement it in a common manner then every user can benefit from it, even if they're not on OSX. thanks -- PMM
Hi, > We *could* do it that way, but it wouldn't look pretty. Instead of having these menu items: > > VGA, Monitor, Serial, Parallel > > We would have these menu items: > > VGA > compat_monitor0 > serial0 > parallel0 > > They look very anti-pretty. Those (except VGA) come from the chardev name. What you list here are simply the default names qemu applies. "Serial" and "serial0" are not that different actually, and attaching a number makes sense as you can have multiple serial lines. Try 'qemu -serial vc -serial vc' and you'll get another entry "serial1". With a more verbose syntax you can have custom names for your chardevs, try 'qemu -chardev vc,id=ttyS0 -device isa-serial,chardev=ttyS0". And IMO it makes sense to have the actual name of the chardev show up in the menu so you know for sure which is which. The 'compat_monitor0' is a bit ugly indeed, IIRC this used to be just 'monitor0' but that caused some backward compatibility issues so it got renamed this way as quick stopgap. When fixing that it would be good to do it in the common code so it is the same for all UIs (as Peter already sayed). cheers, Gerd
diff --git a/ui/cocoa.m b/ui/cocoa.m index d37c29b..8760fb0 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -64,6 +64,8 @@ static int last_buttons; int gArgc; char **gArgv; +int graphics_console, monitor_console, serial_console, parallel_console; + // keymap conversion int keymap[] = @@ -801,6 +803,10 @@ QemuCocoaView *cocoaView; - (void)toggleFullScreen:(id)sender; - (void)showQEMUDoc:(id)sender; - (void)showQEMUTec:(id)sender; +- (void)displayVGA:(id)sender; +- (void)displayMonitor:(id)sender; +- (void)displayParallel:(id)sender; +- (void)displaySerial:(id)sender; @end @implementation QemuCocoaAppController @@ -943,8 +949,32 @@ QemuCocoaView *cocoaView; [[NSWorkspace sharedWorkspace] openFile:[NSString stringWithFormat:@"%@/../doc/qemu/qemu-tech.html", [[NSBundle mainBundle] resourcePath]] withApplication:@"Help Viewer"]; } -@end +/* Displays the VGA screen */ +- (void)displayVGA:(id)sender +{ + console_select(graphics_console); +} + +/* Displays the QEMU Monitor screen */ +- (void)displayMonitor:(id)sender +{ + console_select(monitor_console); +} + +/* Displays the serial port screen */ +- (void)displaySerial:(id)sender +{ + console_select(serial_console); +} + +/* Displays the parallel port screen */ +- (void)displayParallel:(id)sender +{ + console_select(parallel_console); +} + +@end int main (int argc, const char * argv[]) { @@ -1003,13 +1033,6 @@ int main (int argc, const char * argv[]) { [[NSApp mainMenu] addItem:menuItem]; [NSApp performSelector:@selector(setAppleMenu:) withObject:menu]; // Workaround (this method is private since 10.4+) - // View menu - menu = [[NSMenu alloc] initWithTitle:@"View"]; - [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(toggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen - menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil keyEquivalent:@""] autorelease]; - [menuItem setSubmenu:menu]; - [[NSApp mainMenu] addItem:menuItem]; - // Window menu menu = [[NSMenu alloc] initWithTitle:@"Window"]; [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Minimize" action:@selector(performMiniaturize:) keyEquivalent:@"m"] autorelease]]; // Miniaturize @@ -1116,6 +1139,57 @@ static const DisplayChangeListenerOps dcl_ops = { .dpy_refresh = cocoa_refresh, }; +// Creates the view menu +static void create_view_menu() +{ + NSMenu * menu; + NSMenuItem * menuItem; + menu = [[NSMenu alloc] initWithTitle:@"View"]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(toggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen + [menu addItem:[NSMenuItem separatorItem]]; //Separator + if(graphics_console != -1) + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"VGA" action:@selector(displayVGA:) keyEquivalent:@""] autorelease]]; // VGA + if(monitor_console != -1) + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"QEMU Monitor" action:@selector(displayMonitor:) keyEquivalent:@""] autorelease]]; // QEMU Monitor + if(serial_console != -1) + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Serial" action:@selector(displaySerial:) keyEquivalent:@""] autorelease]]; // Serial + if(parallel_console != -1) + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Parallel" action:@selector(displayParallel:) keyEquivalent:@""] autorelease]]; // Parallel + menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil keyEquivalent:@""] autorelease]; + [menuItem setSubmenu:menu]; + [[NSApp mainMenu] insertItem: menuItem atIndex: 1]; // insert View menu after Application menu +} + +// sets the console variables to the actual console' indexes +static void init_console_variables() +{ + int index = 0; + char * console_name; + + // set the console variables to -1 so we know which ones do not exist + graphics_console = -1; + monitor_console = -1; + serial_console = -1; + parallel_console = -1; + + // set the console variables for the consoles we have + while(qemu_console_lookup_by_index(index) != NULL) { + console_name = qemu_console_get_label(qemu_console_lookup_by_index(index)); + if(strstr(console_name, "VGA") != NULL) { + graphics_console = index; + } else if (strstr(console_name, "monitor") != NULL) { + monitor_console = index; + } else if (strstr(console_name, "serial") != NULL) { + serial_console = index; + } else if (strstr(console_name, "parallel") != NULL) { + parallel_console = index; + } else { + printf("Error in initConsoleVariables(): given value %s\n", console_name); + } + index++; + } +} + void cocoa_display_init(DisplayState *ds, int full_screen) { COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n"); @@ -1128,4 +1202,8 @@ void cocoa_display_init(DisplayState *ds, int full_screen) // register cleanup function atexit(cocoa_cleanup); + + // QEMU needs to be running first before we can create the view menu + init_console_variables(); + create_view_menu(); }
This patch adds the VGA, Monitor, Serial, and Parallel menu item to the view menu. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- Removed all code added in console.c. Used existing console code in place of new console code. Added several console global variables to keep track of usable consoles. Simplified console switching code. ui/cocoa.m | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 86 insertions(+), 8 deletions(-)