Hey!

----- Original Message -----
> From: "Pavel Grunt" <[email protected]>
> To: "Lukas Venhoda" <[email protected]>
> Cc: [email protected]
> Sent: Thursday, April 16, 2015 11:55:05 AM
> Subject: Re: [virt-tools-list] [virt-viewer][PATCH] app/window: Set display 
> menu      not sensitive when needed
> 
> Hi, it looks good to me. Just one comment below.
> 
> > 
> > Displays menu must be sensitive only when at least one display is
> > enabled.
> > ---
> >  src/virt-viewer-app.c    |  6 +++++-
> >  src/virt-viewer-window.c | 13 +++++++++++++
> >  src/virt-viewer-window.h |  1 +
> >  3 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > index 563117a..7cf0c60 100644
> > --- a/src/virt-viewer-app.c
> > +++ b/src/virt-viewer-app.c
> > @@ -2253,17 +2253,21 @@ window_update_menu_displays_cb(gpointer
> > value,
> >      GtkMenuShell *submenu;
> >      GList *keys = g_hash_table_get_keys(self->priv->displays);
> >      GList *tmp;
> > +    gboolean sensitive;
> >  
> >      keys = g_list_sort(keys, update_menu_displays_sort);
> >      submenu =
> >      window_empty_display_submenu(VIRT_VIEWER_WINDOW(value));
> >  
> > +    sensitive = (keys != NULL) ? TRUE : FALSE;
> > +
> 
> no need for the ternary operator.

I'm going to disagree here ... for me it's cleaner having the sensitive var 
here.

Apart from that, would be nice to have explicit on the commit message that 
zoom-level changed its behavior for when we have no displays enabled.
It is not a problem for me, but it may be for someone's else (Jonathon, can you 
express your feelings about this?)

> 
> Pavel
> 
> _______________________________________________
> virt-tools-list mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 

Best Regards,
--
Fabiano FidĂȘncio

_______________________________________________
virt-tools-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/virt-tools-list

Reply via email to