[Qemu-devel] [PATCH v4] ui/cocoa.m: Adds console items to the View menu

[Qemu-devel] [PATCH v4] ui/cocoa.m: Adds console items to the View menu

[Programmingkid]

[-- Attachment #1: Type: text/plain, Size: 5851 bytes --]

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(-)

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();
 }
-- 
1.7.5.4


[-- Attachment #2: Type: text/html, Size: 28448 bytes --]

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

[Peter Maydell]

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

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

[Programmingkid]

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.

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

[Peter Maydell]

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

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

[Programmingkid]

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.

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

[Peter Maydell]

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

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

[Gerd Hoffmann]

  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