[virt-tools-list] [PATCH virt-viewer 1/2] Fix rebuild of accelerators menu when loading from file
Jonathon Jongsma
jjongsma at redhat.com
Tue Jan 7 18:12:08 UTC 2014
----- Original Message -----
> From: "Marc-André Lureau" <mlureau at redhat.com>
> To: "Jonathon Jongsma" <jjongsma at redhat.com>
> Cc: "Marc-André Lureau" <marcandre.lureau at gmail.com>, virt-tools-list at redhat.com
> Sent: Tuesday, January 7, 2014 11:48:11 AM
> Subject: Re: [virt-tools-list] [PATCH virt-viewer 1/2] Fix rebuild of accelerators menu when loading from file
>
>
>
> ----- Original Message -----
> > This new set_enable_accel function should not really be necessary.
> > g_object_set() will automatically emit the 'notify' even if the
> > set_property vfunc doesn't explicitly call g_object_notify(). I suspect
> > that
> > the actual bug fix is due solely to the last hunk of the patch in
> > virt_viewer_file_fill_app() (i.e. setting the enable-accel property to true
> > *after* changing the accels instead of before). If that's the case, I'd
> > rather omit the new utility function from the patch and continue using the
> > gobject properties.
>
> That's quite surprising. So there is no way to prevent a notify when calling
> g_object_set() ? weird.
There doesn't appear to be a way to prevent the notify from happening as far as I can see. I guess it's up to the observers to make sure that if they do the right then when they get a notify for a value that hasn't changed...
> In any case, I prefer the explicit version with a dedicated method, so I
> would rather keep the setter.
I guess I don't care too much if you want to keep the utility function. In that case, ACK both patches.
Jonathon
>
> > Jonathon
> >
> >
> > ----- Original Message -----
> > > From: "Marc-André Lureau" <marcandre.lureau at gmail.com>
> > > To: virt-tools-list at redhat.com
> > > Sent: Tuesday, January 7, 2014 7:42:39 AM
> > > Subject: [virt-tools-list] [PATCH virt-viewer 1/2] Fix rebuild of
> > > accelerators menu when loading from file
> > >
> > > It's not enough to set the property to notify of its change. Add a
> > > virt_viewer_app_set_enable_accel() helper, and call it after the changes
> > > to accelerators are made when loading from file.
> > >
> > > I verified the menu is correctly built when connection from controller
> > > or command line too.
> > > ---
> > > src/virt-viewer-app.c | 13 ++++++++++---
> > > src/virt-viewer-app.h | 1 +
> > > src/virt-viewer-file.c | 3 ++-
> > > 3 files changed, 13 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > > index e4ffe4e..4a61d48 100644
> > > --- a/src/virt-viewer-app.c
> > > +++ b/src/virt-viewer-app.c
> > > @@ -1476,7 +1476,7 @@ virt_viewer_app_set_property (GObject *object,
> > > guint
> > > property_id,
> > > break;
> > >
> > > case PROP_ENABLE_ACCEL:
> > > - priv->enable_accel = g_value_get_boolean(value);
> > > + virt_viewer_app_set_enable_accel(self,
> > > g_value_get_boolean(value));
> > > break;
> > >
> > > case PROP_KIOSK:
> > > @@ -1840,6 +1840,13 @@ virt_viewer_app_clear_hotkeys(VirtViewerApp *self)
> > > }
> > >
> > > void
> > > +virt_viewer_app_set_enable_accel(VirtViewerApp *self, gboolean enable)
> > > +{
> > > + self->priv->enable_accel = enable;
> > > + g_object_notify(G_OBJECT(self), "enable-accel");
> > > +}
> > > +
> > > +void
> > > virt_viewer_app_set_hotkeys(VirtViewerApp *self, const gchar
> > > *hotkeys_str)
> > > {
> > > gchar **hotkey, **hotkeys = NULL;
> > > @@ -1851,12 +1858,12 @@ virt_viewer_app_set_hotkeys(VirtViewerApp *self,
> > > const gchar *hotkeys_str)
> > >
> > > if (!hotkeys || g_strv_length(hotkeys) == 0) {
> > > g_strfreev(hotkeys);
> > > - g_object_set(self, "enable-accel", FALSE, NULL);
> > > + virt_viewer_app_set_enable_accel(self, FALSE);
> > > return;
> > > }
> > >
> > > virt_viewer_app_clear_hotkeys(self);
> > > - g_object_set(self, "enable-accel", TRUE, NULL);
> > > + virt_viewer_app_set_enable_accel(self, TRUE);
> > >
> > > for (hotkey = hotkeys; *hotkey != NULL; hotkey++) {
> > > gchar *key = strstr(*hotkey, "=");
> > > diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
> > > index 3ee1f0a..7c77957 100644
> > > --- a/src/virt-viewer-app.h
> > > +++ b/src/virt-viewer-app.h
> > > @@ -102,6 +102,7 @@ void virt_viewer_app_clear_hotkeys(VirtViewerApp
> > > *app);
> > > gint virt_viewer_app_get_n_initial_displays(VirtViewerApp* self);
> > > gint virt_viewer_app_get_initial_monitor_for_display(VirtViewerApp*
> > > self,
> > > gint display);
> > > void virt_viewer_app_set_uuid_string(VirtViewerApp* self, const gchar*
> > > uuid_string);
> > > +void virt_viewer_app_set_enable_accel(VirtViewerApp *app, gboolean
> > > enable);
> > >
> > > G_END_DECLS
> > >
> > > diff --git a/src/virt-viewer-file.c b/src/virt-viewer-file.c
> > > index 238ffea..639d96e 100644
> > > --- a/src/virt-viewer-file.c
> > > +++ b/src/virt-viewer-file.c
> > > @@ -636,7 +636,6 @@ virt_viewer_file_fill_app(VirtViewerFile* self,
> > > VirtViewerApp *app, GError **err
> > >
> > >
> > > virt_viewer_app_clear_hotkeys(app);
> > > - g_object_set(G_OBJECT(app), "enable-accel", TRUE, NULL);
> > >
> > > {
> > > gchar *val;
> > > @@ -661,6 +660,8 @@ virt_viewer_file_fill_app(VirtViewerFile* self,
> > > VirtViewerApp *app, GError **err
> > > }
> > > }
> > >
> > > + virt_viewer_app_set_enable_accel(app, TRUE);
> > > +
> > > if (virt_viewer_file_is_set(self, "fullscreen"))
> > > g_object_set(G_OBJECT(app), "fullscreen",
> > > virt_viewer_file_get_fullscreen(self), NULL);
> > > --
> > > 1.8.4.2
> > >
> > > _______________________________________________
> > > virt-tools-list mailing list
> > > virt-tools-list at redhat.com
> > > https://www.redhat.com/mailman/listinfo/virt-tools-list
> > >
> >
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
>
More information about the virt-tools-list
mailing list