[virt-tools-list] [virt-viewer][PATCH] app/window: Set display menu not sensitive when needed

Pavel Grunt pgrunt at redhat.com
Thu Apr 16 10:42:30 UTC 2015


> 
> Hey!
> 
> ----- Original Message -----
> > From: "Pavel Grunt" <pgrunt at redhat.com>
> > To: "Lukas Venhoda" <lvenhoda at redhat.com>
> > Cc: virt-tools-list at redhat.com
> > 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.
> 

I meant that 'sensitive = (keys != NULL);' is enough.

>
> 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
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > 
> 
> Best Regards,
> --
> Fabiano Fidêncio
> 




More information about the virt-tools-list mailing list