[virt-tools-list] [PATCHv2 virt-viewer 01/10] Try to share more GOption code between r-v and v-v

Marc-André Lureau mlureau at redhat.com
Mon Aug 19 19:48:17 UTC 2013



----- Mensaje original -----
> Hey,
> 
> This generally looks good, 2 comments below:
> 
> On Fri, Aug 16, 2013 at 09:47:37PM +0200, Marc-André Lureau wrote:
> > @@ -105,14 +83,9 @@ main(int argc, char **argv)
> >      GOptionContext *context;
> >      GError *error = NULL;
> >      int ret = 1;
> > -    int zoom = 100;
> >      gchar **args = NULL;
> >      gchar *uri = NULL;
> >      char *title = NULL;
> > -    char *hotkeys = NULL;
> > -    gboolean verbose = FALSE;
> > -    gboolean debug = FALSE;
> > -    gboolean direct = FALSE;
> 
> You seem to have squashed your 'remote-viewer: remove -d direct option'
> patch into that one.

kinda, next rebase will have this solved anyway.

> 
> >  #ifdef HAVE_GTK_VNC
> > @@ -186,22 +148,13 @@ main(int argc, char **argv)
> >          }
> >      }
> >  
> > -    if (zoom < 10 || zoom > 200) {
> > -        g_printerr(_("Zoom level must be within 10-200\n"));
> > -        goto cleanup;
> > -    }
> 
> Max zoom is 200 here...
> 
> > diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
> > index 93bb988..4721fe3 100644
> > --- a/src/virt-viewer-app.h
> > +++ b/src/virt-viewer-app.h
> > @@ -120,21 +106,10 @@ int main(int argc, char **argv)
> >          goto cleanup;
> >      }
> >  
> > -    if (zoom < 10 || zoom > 200) {
> > -        g_printerr(_("Zoom level must be within 10-200\n"));
> > -        goto cleanup;
> > -    }
> 
> ...and here too...
> 
> > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > index 6d12584..13c2565 100644
> > --- a/src/virt-viewer-app.c
> > +++ b/src/virt-viewer-app.c
> > @@ -1419,6 +1429,14 @@ virt_viewer_app_init (VirtViewerApp *self)
> >          g_debug("Couldn't load configuration: %s", error->message);
> >  
> >      g_clear_error(&error);
> > +
> > +    if (opt_zoom < 10 || opt_zoom > 400) {
> > +        g_printerr(_("Zoom level must be within 10-400\n"));
> > +        opt_zoom = 100;
> > +    }
> 
> ...and it becomes 400 here. I'm fine with either value, but it would be
> better if this patch was only a move/factoring of code.

I was making this consistant with the max value accepted by virt-viewer-window.

I'll split it.




More information about the virt-tools-list mailing list