[virt-tools-list] [virt-viewer 3/4] Reshow connection dialog on errors

[Jonathon Jongsma]

Oops, forgot to cc virt-tools-list earlier...

----- Original Message -----
> From: "Christophe Fergeau" <cfergeau at redhat.com>
> To: "Jonathon Jongsma" <jjongsma at redhat.com>
> Sent: Wednesday, October 30, 2013 5:27:18 AM
> Subject: Re: [virt-tools-list] [virt-viewer 3/4] Reshow connection dialog on errors
> 
> Hey,
> 
> Don't know if you excluded the mailing list on purpose?
> 
> On Tue, Oct 29, 2013 at 12:44:57PM -0400, Jonathon Jongsma wrote:
> > Can this be abstracted out into a separate function or something instead of
> > jumping around with gotos?
> > 
> > e.g.
> > 
> > do {
> >     ret = remote_viewer_start_internal(self, guri);
> > } while (!ret && priv->open_recent_dialog)
> > 
> > Or something of that sort? It's up to you, but I generally find that
> > functions with more than a single goto become more cumbersome to maintain.
> 
> Yeah, I agree with you, though in this case it's actually quite awkward, as
> the internal function would basically need to be tristate, everything was
> OK, there was an error, or the user cancelled. Maybe there's an easier way,
> but the diff I ended up was much bigger than that one /o\


Yes, understood.  I didn't actually investigate how feasible it was to change, so if it's not simple, go ahead with the current approach.

jonathon



> 
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 3b7b804..2454b2b 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -942,16 +942,22 @@ connect_dialog(gchar **uri)
>      return retval;
>  }
>  
> -static gboolean
> -remote_viewer_start(VirtViewerApp *app)
> +enum RemoteViewerStartStatus {
> +    START_OK,
> +    START_CANCELLED,
> +    START_ERROR
> +};
> +
> +static enum RemoteViewerStartStatus
> +remote_viewer_start_internal(VirtViewerApp *app)
>  {
> -    g_return_val_if_fail(REMOTE_VIEWER_IS(app), FALSE);
> +    g_return_val_if_fail(REMOTE_VIEWER_IS(app), START_ERROR);
>  
>      RemoteViewer *self = REMOTE_VIEWER(app);
>      RemoteViewerPrivate *priv = self->priv;
>      GFile *file = NULL;
>      VirtViewerFile *vvfile = NULL;
> -    gboolean ret = FALSE;
> +    enum RemoteViewerStartStatus ret = START_ERROR;
>      gchar *guri = NULL;
>      gchar *type = NULL;
>      GError *error = NULL;
> @@ -981,12 +987,12 @@ remote_viewer_start(VirtViewerApp *app)
>  retry_dialog:
>          if (priv->open_recent_dialog) {
>              if (connect_dialog(&guri) != 0)
> -                return FALSE;
> +                return START_CANCELLED;
>              g_object_set(app, "guri", guri, NULL);
>          } else
>              g_object_get(app, "guri", &guri, NULL);
>  
> -        g_return_val_if_fail(guri != NULL, FALSE);
> +        g_return_val_if_fail(guri != NULL, START_ERROR);
>  
>          DEBUG_LOG("Opening display to %s", guri);
>          if ((virt_viewer_app_get_title(app) == NULL) || priv->default_title)
>          {
> @@ -1039,7 +1045,11 @@ retry_dialog:
>      }
>  #endif
>  
> -    ret = VIRT_VIEWER_APP_CLASS(remote_viewer_parent_class)->start(app);
> +    if (VIRT_VIEWER_APP_CLASS(remote_viewer_parent_class)->start(app)) {
> +        ret = START_OK;
> +    } else {
> +        ret = START_ERROR;
> +    }
>  
>  cleanup:
>      g_clear_object(&file);
> @@ -1054,6 +1064,20 @@ cleanup:
>      return ret;
>  }
>  
> +static gboolean
> +remote_viewer_start(VirtViewerApp *app)
> +{
> +    RemoteViewer *self = REMOTE_VIEWER(app);
> +    RemoteViewerPrivate *priv = self->priv;
> +    enum RemoteViewerStartStatus ret;
> +
> +    do {
> +            ret = remote_viewer_start_internal(app);
> +    } while ((ret == START_ERROR) && priv->open_recent_dialog);
> +
> +   return (ret == START_OK);
> +}
> +
>  /*
>   * Local variables:
>   *  c-indent-level: 4
> 
>