[virt-tools-list] [PATCH virt-viewer 05/10] remote-viewer: Make ovirt-foreign-menu a property

[Eduardo Lima (Etrunko)]

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 at redhat.com>
>> ---
>>  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.

> 
>> +    return dialog;
>>  }
>> diff --git a/src/remote-viewer-iso-list-dialog.h b/src/remote-viewer-iso-list-dialog.h
>> index def841b..8b936f5 100644
>> --- a/src/remote-viewer-iso-list-dialog.h
>> +++ b/src/remote-viewer-iso-list-dialog.h
>> @@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass
>>  
>>  GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST;
>>  
>> -GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent);
>> +GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu);
>>  
>>  G_END_DECLS
>>  
> 
> The remote-viewer-iso-list-dialog.[ch] changes are odd in this patch,
> they don't have much to do with the added
> RemoteViewer::ovirt-foreign-menu property at this point.
> I think this part of the patch can probably be merged with the previous
> one, the property addition can be done before that.
> 

Thanks for the review.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170118/c56ab4e1/attachment.sig>