[virt-tools-list] [virt-manager PATCH 2/2] console: don't use _close_viewer directly

[Cole Robinson]

On 08/17/2016 12:38 PM, Pavel Hrdina wrote:
> Function _close_viewer should be always called together with setting
> unavailable page.  Call only _activate_unavailable_page which does both things.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  virtManager/console.py | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/virtManager/console.py b/virtManager/console.py
> index 6e3d4eb..3f8cfb9 100644
> --- a/virtManager/console.py
> +++ b/virtManager/console.py
> @@ -601,7 +601,7 @@ class vmmConsolePages(vmmGObjectUI):
>      # Page Navigation #
>      ###################
>  
> -    def _activate_unavailable_page(self, msg):
> +    def _activate_unavailable_page(self, msg=None):
>          """
>          This function is passed to serialcon.py at least, so change
>          with care
> @@ -609,7 +609,8 @@ class vmmConsolePages(vmmGObjectUI):
>          self._close_viewer()
>          self.widget("console-pages").set_current_page(
>              _CONSOLE_PAGE_UNAVAILABLE)
> -        self.widget("console-unavailable").set_label("<b>" + msg + "</b>")
> +        if msg:
> +            self.widget("console-unavailable").set_label("<b>" + msg + "</b>")
> 

Rather than the msg=None handling I'd rather have callers always pass a
string, otherwise future code re-org might end up showing an empty screen to
the user. Can you change the msg=None callers to use something like _("Viewer
disconnected.") ? Even though users shouldn't see it. ACK both with that
change if you agree

Thanks,
Cole

>      def _activate_auth_page(self, withPassword, withUsername):
>          (pw, username) = self.config.get_console_password(self.vm)
> @@ -786,7 +787,6 @@ class vmmConsolePages(vmmGObjectUI):
>              self._enable_modifiers()
>  
>      def _viewer_auth_rejected(self, ignore, errmsg):
> -        self._close_viewer()
>          self._activate_unavailable_page(errmsg)
>  
>      def _viewer_auth_error(self, ignore, errmsg, viewer_will_disconnect):
> @@ -797,7 +797,6 @@ class vmmConsolePages(vmmGObjectUI):
>              # GtkVNC will disconnect after an auth error, so lets do it for
>              # them and re-init the viewer (which will be triggered by
>              # update_vm_widget_states if needed)
> -            self._close_viewer()
>              self._activate_unavailable_page(errmsg)
>  
>          self._update_vm_widget_states()
> @@ -829,9 +828,7 @@ class vmmConsolePages(vmmGObjectUI):
>          self._activate_unavailable_page(msg)
>  
>      def _viewer_disconnected(self, ignore, errdetails, ssherr):
> -        self.widget("console-pages").set_current_page(
> -            _CONSOLE_PAGE_UNAVAILABLE)
> -        self._close_viewer()
> +        self._activate_unavailable_page()
>          logging.debug("Viewer disconnected")
>  
>          # Make sure modifiers are set correctly
> @@ -1016,9 +1013,7 @@ class vmmConsolePages(vmmGObjectUI):
>          return self._viewer.console_get_pixbuf()
>  
>      def details_close_viewer(self):
> -        self.widget("console-pages").set_current_page(
> -                _CONSOLE_PAGE_UNAVAILABLE)
> -        return self._close_viewer()
> +        return self._activate_unavailable_page()
>  
>      def details_activate_default_console_page(self):
>          return self._activate_default_console_page()
>