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

Christophe Fergeau cfergeau at redhat.com
Mon Aug 19 18:31:11 UTC 2013


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.

>  #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.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20130819/87c21fea/attachment.sig>


More information about the virt-tools-list mailing list