console: Text console submenu improvements

* Move all the menu building to its own class, for clarity
* Rename the menu 'Consoles' since it contains graphical choice as well
* Strip out the VM console duplicate if it exists
* Simplify the code a bit

Signed-off-by: Cole Robinson <crobinso@redhat.com>

tests/uitests/test_details.py

@@ -356,7 +356,7 @@ class Details(uiutils.UITestCase):
         # Quick test to hit some serialcon.py paths
         viewmenu = win.find("^View$", "menu")
         viewmenu.click()
-        textmenu = viewmenu.find("Text Consoles", "menu")
+        textmenu = viewmenu.find("Consoles", "menu")
         textmenu.point()
         conitem = textmenu.find("Serial 1")
         uiutils.check(lambda: not conitem.sensitive)
@@ -753,9 +753,9 @@ class Details(uiutils.UITestCase):
         def _check_textconsole_menu(msg):
             vmenu = win.find("^View$", "menu")
             vmenu.click()
-            tmenu = win.find("Text Consoles", "menu")
+            tmenu = win.find("Consoles", "menu")
             tmenu.point()
-            tmenu.find(msg, "menu item")
+            tmenu.find(msg, ".*menu item.*")
             vmenu.click()
 
         # Check initial state

tests/uitests/test_livetests.py

@@ -233,12 +233,12 @@ class Console(uiutils.UITestCase):
         def _click_textconsole_menu(msg):
             vmenu = win.find("^View$", "menu")
             vmenu.click()
-            tmenu = win.find("Text Consoles", "menu")
+            tmenu = win.find("Consoles", "menu")
             tmenu.point()
             tmenu.find(msg, "radio menu item").click()
 
         # A bit of an extra test, make sure selecting Graphical Console works
-        _click_textconsole_menu("Text Console 1")
+        _click_textconsole_menu("Serial 1")
         uiutils.check(lambda: not con.showing)
         _click_textconsole_menu("Graphical Console")
         uiutils.check(lambda: con.showing)
@@ -274,7 +274,7 @@ class Console(uiutils.UITestCase):
         view = self.app.root.find("^View$", "menu")
         view.click()
         # Triggers some tooltip cases
-        textmenu = view.find("Text Consoles", "menu")
+        textmenu = view.find("Consoles", "menu")
         textmenu.point()
         uiutils.check(lambda: textmenu.showing)
         item = textmenu.find("Text Console 1")

ui/vmwindow.ui

@@ -244,10 +244,10 @@
                       </object>
                     </child>
                     <child>
-                      <object class="GtkMenuItem" id="details-menu-view-serial-list">
+                      <object class="GtkMenuItem" id="details-menu-view-console-list">
                         <property name="visible">True</property>
                         <property name="can_focus">False</property>
-                        <property name="label" translatable="yes">_Text Consoles</property>
+                        <property name="label" translatable="yes">Co_nsoles</property>
                         <property name="use_underline">True</property>
                       </object>
                     </child>

virtManager/details/console.py

@@ -185,6 +185,111 @@ class vmmOverlayToolbar:
         self._send_key_button.set_sensitive(can_sendkey)
 
 
+class _ConsoleMenu:
+    """
+    Helper class for building the text/graphical console menu list
+    """
+
+    ################
+    # Internal API #
+    ################
+
+    def _build_serial_menu_items(self, vm):
+        devs = vmmSerialConsole.get_serialcon_devices(vm)
+        if len(devs) == 0:
+            return [[_("No text console available"), None, None]]
+
+        ret = []
+        for dev in devs:
+            if dev.DEVICE_TYPE == "console":
+                label = _("Text Console %d") % (dev.get_xml_idx() + 1)
+            else:
+                label = _("Serial %d") % (dev.get_xml_idx() + 1)
+
+            tooltip = vmmSerialConsole.can_connect(vm, dev)
+            ret.append([label, dev, tooltip])
+        return ret
+
+    def _build_graphical_menu_items(self, vm):
+        devs = vm.xmlobj.devices.graphics
+        if len(devs) == 0:
+            return [[_("No graphical console available"), None, None]]
+
+        from ..device.gfxdetails import vmmGraphicsDetails
+
+        ret = []
+        for idx, dev in enumerate(devs):
+            label = (_("Graphical Console") + " " +
+                     vmmGraphicsDetails.graphics_pretty_type_simple(dev.type))
+
+            tooltip = None
+            if idx > 0:
+                label += " %s" % (idx + 1)
+                tooltip = _("virt-manager does not support more "
+                            "than one graphical console")
+
+            ret.append([label, dev, tooltip])
+        return ret
+
+
+    ##############
+    # Public API #
+    ##############
+
+    def rebuild_menu(self, vm, submenu, toggled_cb):
+        oldlabel = None
+        for child in submenu.get_children():
+            if hasattr(child, 'get_active') and child.get_active():
+                oldlabel = child.get_label()
+            submenu.remove(child)
+
+        graphics = self._build_graphical_menu_items(vm)
+        serials = self._build_serial_menu_items(vm)
+
+        # Use label == None to tell the loop to add a separator
+        items = graphics + [[None, None, None]] + serials
+
+        last_item = None
+        for (label, dev, tooltip) in items:
+            if label is None:
+                submenu.add(Gtk.SeparatorMenuItem())
+                continue
+
+            cb = toggled_cb
+            cbdata = dev
+            sensitive = dev and not tooltip
+
+            active = False
+            if oldlabel is None and sensitive:
+                # Select the first selectable option
+                oldlabel = label
+            if label == oldlabel:
+                active = True
+
+            item = Gtk.RadioMenuItem()
+            if last_item is None:
+                last_item = item
+            else:
+                item.join_group(last_item)
+
+            item.set_label(label)
+            item.set_active(active and sensitive)
+            if cbdata and sensitive:
+                item.connect("toggled", cb, cbdata)
+
+            item.set_sensitive(sensitive)
+            item.set_tooltip_text(tooltip or None)
+            submenu.add(item)
+
+        submenu.show_all()
+
+    def activate_default(self, menu):
+        for child in menu.get_children():
+            if child.get_sensitive() and hasattr(child, "toggled"):
+                child.toggled()
+                break
+
+
 class vmmConsolePages(vmmGObjectUI):
     """
     Handles all the complex UI handling dictated by the spice/vnc widgets
@@ -222,6 +327,7 @@ class vmmConsolePages(vmmGObjectUI):
         self.widget("console-pages").set_show_tabs(False)
         self.widget("serial-pages").set_show_tabs(False)
 
+        self._consolemenu = _ConsoleMenu()
         self._serial_consoles = []
         self._init_menus()
 
@@ -268,7 +374,7 @@ class vmmConsolePages(vmmGObjectUI):
         # Serial list menu
         smenu = Gtk.Menu()
         smenu.connect("show", self._populate_serial_menu)
-        self.widget("details-menu-view-serial-list").set_submenu(smenu)
+        self.widget("details-menu-view-console-list").set_submenu(smenu)
 
         # Keycombo menu (ctrl+alt+del etc.)
         self.widget("details-menu-send-key").set_submenu(self._keycombo_menu)
@@ -866,28 +972,18 @@ class vmmConsolePages(vmmGObjectUI):
 
     def _activate_default_console_page(self):
         """
-        Find the default graphical or serial console for the VM
+        Toggle default console page from the menu
         """
-        if (self.vm.xmlobj.devices.graphics or
-            not self.vm.get_serialcon_devices()):
-            return
-
         # We iterate through the 'console' menu and activate the first
-        # valid entry... it's the easiest thing to do to hit all the right
+        # valid entry... hacky but it works
-        # code paths.
         self._populate_serial_menu()
-        menu = self.widget("details-menu-view-serial-list").get_submenu()
+        menu = self.widget("details-menu-view-console-list").get_submenu()
-        for child in menu.get_children():
+        self._consolemenu.activate_default(menu)
-            if isinstance(child, Gtk.SeparatorMenuItem):
-                break  # pragma: no cover
-            if child.get_sensitive():
-                child.toggled()
-                break
 
     def _console_menu_toggled(self, src, dev):
         self.widget("details-pages").set_current_page(DETAILS_PAGE_CONSOLE)
 
-        if dev.DEVICE_TYPE == "graphics":
+        if dev and dev.DEVICE_TYPE == "graphics":
             self.widget("console-pages").set_current_page(_CONSOLE_PAGE_VIEWER)
             return
 
@@ -913,89 +1009,11 @@ class vmmConsolePages(vmmGObjectUI):
         self.widget("console-pages").set_current_page(_CONSOLE_PAGE_SERIAL)
         self.widget("serial-pages").set_current_page(page_idx)
 
-    def _build_serial_menu_items(self, menu_item_cb):
-        devs = self.vm.get_serialcon_devices()
-        if len(devs) == 0:
-            menu_item_cb(_("No text console available"),
-                         radio=False, sensitive=False)
-            return
-
-        active_label = None
-        if (self.widget("console-pages").get_current_page() ==
-                _CONSOLE_PAGE_SERIAL):
-            serial_page = self.widget("serial-pages").get_current_page()
-            if len(self._serial_consoles) > serial_page:
-                active_label = self._serial_consoles[serial_page].name
-
-        for dev in devs:
-            if dev.DEVICE_TYPE == "console":
-                label = _("Text Console %d") % (dev.get_xml_idx() + 1)
-            else:
-                label = _("Serial %d") % (dev.get_xml_idx() + 1)
-
-            tooltip = vmmSerialConsole.can_connect(self.vm, dev)
-            sensitive = not bool(tooltip)
-
-            active = (sensitive and label == active_label)
-            menu_item_cb(label, sensitive=sensitive, active=active,
-                tooltip=tooltip, cb=self._console_menu_toggled, cbdata=dev)
-
-    def _build_graphical_menu_items(self, menu_item_cb):
-        devs = self.vm.xmlobj.devices.graphics
-        if len(devs) == 0:
-            menu_item_cb(_("No graphical console available"),
-                         radio=False, sensitive=False)
-            return
-
-        from ..device.gfxdetails import vmmGraphicsDetails
-
-        active = (self.widget("console-pages").get_current_page() !=
-                _CONSOLE_PAGE_SERIAL)
-        for idx, dev in enumerate(devs):
-            label = (_("Graphical Console") + " " +
-                     vmmGraphicsDetails.graphics_pretty_type_simple(dev.type))
-
-            sensitive = True
-            tooltip = None
-            if idx > 0:
-                label += " %s" % (idx + 1)
-                sensitive = False
-                tooltip = _("virt-manager does not support more "
-                            "that one graphical console")
-
-            menu_item_cb(label, active=active,
-                sensitive=sensitive, tooltip=tooltip,
-                cb=self._console_menu_toggled, cbdata=dev)
-
     def _populate_serial_menu(self, ignore=None):
-        src = self.widget("details-menu-view-serial-list").get_submenu()
+        submenu = self.widget("details-menu-view-console-list").get_submenu()
-        for child in src:
+        self._consolemenu.rebuild_menu(
-            src.remove(child)
+                self.vm, submenu, self._console_menu_toggled)
 
-        def menu_item_cb(label, sensitive=True, active=False,
-                         radio=True, tooltip=None, cb=None, cbdata=None):
-            if radio:
-                item = Gtk.RadioMenuItem(menu_item_cb.radio_group)
-                if menu_item_cb.radio_group is None:
-                    menu_item_cb.radio_group = item
-                item.set_label(label)
-            else:
-                item = Gtk.MenuItem.new_with_label(label)
-
-            item.set_sensitive(sensitive)
-            if active:
-                item.set_active(True)
-            if tooltip:
-                item.set_tooltip_text(tooltip)
-            if cb and sensitive:
-                item.connect("toggled", cb, cbdata)
-            src.add(item)
-        menu_item_cb.radio_group = None
-
-        self._build_serial_menu_items(menu_item_cb)
-        src.add(Gtk.SeparatorMenuItem())
-        self._build_graphical_menu_items(menu_item_cb)
-        src.show_all()
 
 
     ###########################

virtManager/details/serialcon.py

@@ -186,6 +186,15 @@ class vmmSerialConsole(vmmGObject):
 
         return err
 
+    @staticmethod
+    def get_serialcon_devices(vm):
+        serials = vm.xmlobj.devices.serial
+        consoles = vm.xmlobj.devices.console
+        if serials and vm.serial_is_console_dup(serials[0]):
+            consoles.pop(0)
+        return serials + consoles
+
+
     def __init__(self, vm, target_port, name):
         vmmGObject.__init__(self)
 
@@ -290,7 +299,7 @@ class vmmSerialConsole(vmmGObject):
         self._box.set_current_page(1)
 
     def _lookup_dev(self):
-        devs = self.vm.get_serialcon_devices()
+        devs = vmmSerialConsole.get_serialcon_devices(self.vm)
         found = None
         for dev in devs:
             port = dev.get_xml_idx()

virtManager/object/domain.py

@@ -1268,8 +1268,6 @@ class vmmDomain(vmmLibvirtObject):
         # used and clutters the UI
         return self.xmlobj.get_bootable_devices(exclude_redirdev=True)
 
-    def get_serialcon_devices(self):
-        return self.xmlobj.devices.serial + self.xmlobj.devices.console
 
     ############################
     # Domain lifecycle methods #