On Wed, Jan 18, 2017 at 10:56:59AM -0200, Eduardo Lima (Etrunko) wrote:
> On 18/01/17 10:53, Christophe Fergeau wrote:
> > On Wed, Jan 18, 2017 at 10:08:42AM -0200, Eduardo Lima (Etrunko) wrote:
> >> On 18/01/17 09:53, Eduardo Lima (Etrunko) wrote:
> >>> On 17/01/17 14:00, Christophe Fergeau wrote:
> >>>> On Fri, Jan 13, 2017 at 07:11:07PM -0200, Eduardo Lima (Etrunko) wrote:
> >>>>> The OvirtForeignMenu pointer is needed by the new ISO list dialog, and
> >>>>> we make it acessible via property to avoid interdependency between
> >>>>> objects.
> >>>>>
> >>>>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx>
> >>>>> ---
> >>>>> src/remote-viewer-iso-list-dialog.c | 31 +++++++++++++++++++++++--------
> >>>>> src/remote-viewer-iso-list-dialog.h | 2 +-
> >>>>> src/remote-viewer.c | 37 +++++++++++++++++++++++++++++++++++++
> >>>>> 3 files changed, 61 insertions(+), 9 deletions(-)
> >>>>>
> >>>>> diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c
> >>>>> index 0fbea28..858719c 100644
> >>>>> --- a/src/remote-viewer-iso-list-dialog.c
> >>>>> +++ b/src/remote-viewer-iso-list-dialog.c
> >>>>> @@ -24,6 +24,7 @@
> >>>>>
> >>>>> #include "remote-viewer-iso-list-dialog.h"
> >>>>> #include "virt-viewer-util.h"
> >>>>> +#include "ovirt-foreign-menu.h"
> >>>>>
> >>>>> G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG)
> >>>>>
> >>>>> @@ -33,11 +34,16 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE
> >>>>> struct _RemoteViewerISOListDialogPrivate
> >>>>> {
> >>>>> GtkWidget *stack;
> >>>>> + OvirtForeignMenu *foreign_menu;
> >>>>> };
> >>>>>
> >>>>> static void
> >>>>> remote_viewer_iso_list_dialog_dispose(GObject *object)
> >>>>> {
> >>>>> + RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
> >>>>> + RemoteViewerISOListDialogPrivate *priv = self->priv;
> >>>>> +
> >>>>> + g_clear_object(&priv->foreign_menu);
> >>>>> G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
> >>>>> }
> >>>>>
> >>>>> @@ -94,13 +100,22 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
> >>>>> }
> >>>>>
> >>>>> GtkWidget *
> >>>>> -remote_viewer_iso_list_dialog_new(GtkWindow *parent)
> >>>>> +remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
> >>>>> {
> >>>>> - return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
> >>>>> - "title", _("Change CD"),
> >>>>> - "transient-for", parent,
> >>>>> - "border-width", 18,
> >>>>> - "default-width", 400,
> >>>>> - "default-height", 300,
> >>>>> - NULL);
> >>>>> + GtkWidget *dialog;
> >>>>> + RemoteViewerISOListDialog *self;
> >>>>> +
> >>>>> + g_return_val_if_fail(foreign_menu != NULL, NULL);
> >>>>> +
> >>>>> + dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
> >>>>> + "title", _("Change CD"),
> >>>>> + "transient-for", parent,
> >>>>> + "border-width", 18,
> >>>>> + "default-width", 400,
> >>>>> + "default-height", 300,
> >>>>> + NULL);
> >>>>> +
> >>>>> + self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
> >>>>> + self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
> >>>>
> >>>> Fwiw, a construct-only GObject property would be more expected (but you
> >>>> can keep it this way i f you think it's better).
> >>>
> >>>
> >>> No objections, I will change it.
> >>
> >>
> >> Thinking better about this, it is not possible to make it a construct
> >> only, because the ovirt object is created later on the process, when
> >> remote-viewer has already started. Also, there is the possibility of the
> >> pointer being set or not, depending on how the application was invoked.
> >
> > I don't understand what you mean here. In the part of the code quoted
> > above, a RemoteViewerISOListDialog instance is created, and right after
> > that, we set self->priv->foreign_menu on that instance, so in this case,
> > a CONSTRUCT_ONLY property would be fine.
>
> The ovirt-foreign-menu property is installed as a member of RemoteViewer
> object, not RemoteViewerISODialog.
Oh, what I was saying is that
+ dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
+ "title", _("Change CD"),
+ "transient-for", parent,
+ "border-width", 18,
+ "default-width", 400,
+ "default-height", 300,
+ NULL);
+
+ self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+ self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
should probably be
+ dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
+ "title", _("Change CD"),
+ "transient-for", parent,
+ "border-width", 18,
+ "default-width", 400,
+ "default-height", 300,
+ "ovirt-foreign-menu", foreign_menu,
+ NULL);
But as I was saying, it's not that important here :)
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
