Re: [virt-tools-list] [PATCH virt-viewer 2/5] Introduce ISO List dialog
On Mon, Jan 23, 2017 at 05:01:34PM -0200, Eduardo Lima (Etrunko) wrote:
> >> +
> >> +static void
> >> +fetch_iso_names_cb(OvirtForeignMenu *foreign_menu,
> >> + GAsyncResult *result,
> >> + RemoteViewerISOListDialog *self)
> >> +{
> >> +RemoteViewerISOListDialogPrivate *priv = self->priv;
> >> +GError *error = NULL;
> >> +GList *iso_list;
> >> +
> >> +iso_list = ovirt_foreign_menu_fetch_iso_names_finish(foreign_menu,
> >> result, &error);
> >> +
> >> +if (!iso_list) {
> >> +const gchar *msg = error ? error->message : _("Failed to fetch CD
> >> names");
> >> +gchar *markup = g_strdup_printf("%s", msg);
> >> +
> >> +gtk_label_set_markup(GTK_LABEL(priv->status), markup);
> >> +gtk_spinner_stop(GTK_SPINNER(priv->spinner));
> >> +remote_viewer_iso_list_dialog_show_error(self, msg);
> >> +gtk_dialog_set_response_sensitive(GTK_DIALOG(self),
> >> GTK_RESPONSE_NONE, TRUE);
> >> +g_free(markup);
> >> +g_clear_error(&error);
> >
> > I'd also close 'self' when the error message is dismissed (so you don't
> > really need to set a label in the iso dialog).
>
> I would keep it like this, because it can be a temporary networking
> problem, and user can hit the reload button right away, instead of going
> to the main menu and opening the dialog again. This happened to me
> during one of my tests, where I had a DNS query failure which did not
> happen the second time.
And for me it's useless as the most common error I get is a certificate
error which is not transient, and I need to close 2 error dialogs
instead of one.
If the use case you want to promote is an easy way to retry in case
there was a temporary network issue, why do we have a "dismiss" button
to click first before hitting refresh, rather than having the dialog
directly offer the option to retry?
Christophe
signature.asc
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer 2/5] Introduce ISO List dialog
On 20/01/17 13:20, Christophe Fergeau wrote:
> Can you expand a bit in the commit log about what this iso dialog is,
> what is your goal for introducing it, .. ? Huge commit with shortlog
> does not look good ;)
>
Alright, it will be done for the next version.
> On Thu, Jan 19, 2017 at 01:42:11PM -0200, Eduardo Lima (Etrunko) wrote:
>> Signed-off-by: Eduardo Lima (Etrunko)
>> ---
>> po/POTFILES.in | 2 +
>> src/Makefile.am| 3 +
>> src/remote-viewer-iso-list-dialog.c| 365
>> +
>> src/remote-viewer-iso-list-dialog.h| 58 +
>> src/resources/ui/remote-viewer-iso-list.ui | 158 +
>> src/resources/virt-viewer.gresource.xml| 1 +
>> 6 files changed, 587 insertions(+)
>> create mode 100644 src/remote-viewer-iso-list-dialog.c
>> create mode 100644 src/remote-viewer-iso-list-dialog.h
>> create mode 100644 src/resources/ui/remote-viewer-iso-list.ui
>>
>> diff --git a/po/POTFILES.in b/po/POTFILES.in
>> index 69d9fef..371c242 100644
>> --- a/po/POTFILES.in
>> +++ b/po/POTFILES.in
>> @@ -1,9 +1,11 @@
>> data/remote-viewer.appdata.xml.in
>> data/remote-viewer.desktop.in
>> data/virt-viewer-mime.xml.in
>> +src/remote-viewer-iso-list-dialog.c
>> src/remote-viewer-main.c
>> src/remote-viewer.c
>> [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui
>> +[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui
>> [type: gettext/glade] src/resources/ui/virt-viewer-about.ui
>> src/virt-viewer-app.c
>> src/virt-viewer-auth.c
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 272c4ff..9748277 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -13,6 +13,7 @@ noinst_DATA = \
>> resources/ui/virt-viewer-vm-connection.ui \
>> resources/ui/virt-viewer-preferences.ui \
>> resources/ui/remote-viewer-connect.ui \
>> +resources/ui/remote-viewer-iso-list.ui \
>> resources/ui/virt-viewer-file-transfer-dialog.ui \
>> $(NULL)
>>
>> @@ -97,6 +98,8 @@ if HAVE_OVIRT
>> libvirt_viewer_la_SOURCES += \
>> ovirt-foreign-menu.h \
>> ovirt-foreign-menu.c \
>> +remote-viewer-iso-list-dialog.c \
>> +remote-viewer-iso-list-dialog.h \
>> $(NULL)
>> endif
>>
>> diff --git a/src/remote-viewer-iso-list-dialog.c
>> b/src/remote-viewer-iso-list-dialog.c
>> new file mode 100644
>> index 000..bf7c6c7
>> --- /dev/null
>> +++ b/src/remote-viewer-iso-list-dialog.c
>> @@ -0,0 +1,365 @@
>> +/*
>> + * Virt Viewer: A virtual machine console viewer
>> + *
>> + * Copyright (C) 2016 Red Hat, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> + */
>> +
>> +#include
>> +
>> +#include
>> +
>> +#include "remote-viewer-iso-list-dialog.h"
>> +#include "virt-viewer-util.h"
>> +#include "ovirt-foreign-menu.h"
>> +
>> +static void ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu
>> *foreign_menu, GAsyncResult *result, RemoteViewerISOListDialog *self);
>> +static void
>> remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self,
>> const gchar *message);
>> +
>> +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog,
>> GTK_TYPE_DIALOG)
>> +
>> +#define DIALOG_PRIVATE(o) \
>> +(G_TYPE_INSTANCE_GET_PRIVATE((o),
>> REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate))
>> +
>> +struct _RemoteViewerISOListDialogPrivate
>> +{
>> +GtkListStore *list_store;
>> +GtkWidget *status;
>> +GtkWidget *spinner;
>> +GtkWidget *stack;
>> +GtkWidget *tree_view;
>> +OvirtForeignMenu *foreign_menu;
>> +};
>> +
>> +enum RemoteViewerISOListDialogModel
>> +{
>> +ISO_IS_ACTIVE = 0,
>> +ISO_NAME,
>> +FONT_WEIGHT,
>> +};
>> +
>> +enum RemoteViewerISOListDialogProperties {
>> +PROP_0,
>> +PROP_FOREIGN_MENU,
>> +};
>> +
>> +
>> +void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle
>> *cell_renderer, gchar *path, gpointer user_data);
>> +void remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view,
>> GtkTreePath *path, GtkTreeViewColumn *col, gpointer user_data);
>> +
>> +static void
>> +remote_viewer_iso_list_dialog_dispose(GObject *object)
>> +{
>> +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(ob
Re: [virt-tools-list] [PATCH virt-viewer 2/5] Introduce ISO List dialog
Can you expand a bit in the commit log about what this iso dialog is,
what is your goal for introducing it, .. ? Huge commit with shortlog
does not look good ;)
On Thu, Jan 19, 2017 at 01:42:11PM -0200, Eduardo Lima (Etrunko) wrote:
> Signed-off-by: Eduardo Lima (Etrunko)
> ---
> po/POTFILES.in | 2 +
> src/Makefile.am| 3 +
> src/remote-viewer-iso-list-dialog.c| 365
> +
> src/remote-viewer-iso-list-dialog.h| 58 +
> src/resources/ui/remote-viewer-iso-list.ui | 158 +
> src/resources/virt-viewer.gresource.xml| 1 +
> 6 files changed, 587 insertions(+)
> create mode 100644 src/remote-viewer-iso-list-dialog.c
> create mode 100644 src/remote-viewer-iso-list-dialog.h
> create mode 100644 src/resources/ui/remote-viewer-iso-list.ui
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 69d9fef..371c242 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -1,9 +1,11 @@
> data/remote-viewer.appdata.xml.in
> data/remote-viewer.desktop.in
> data/virt-viewer-mime.xml.in
> +src/remote-viewer-iso-list-dialog.c
> src/remote-viewer-main.c
> src/remote-viewer.c
> [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui
> +[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui
> [type: gettext/glade] src/resources/ui/virt-viewer-about.ui
> src/virt-viewer-app.c
> src/virt-viewer-auth.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 272c4ff..9748277 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -13,6 +13,7 @@ noinst_DATA = \
> resources/ui/virt-viewer-vm-connection.ui \
> resources/ui/virt-viewer-preferences.ui \
> resources/ui/remote-viewer-connect.ui \
> + resources/ui/remote-viewer-iso-list.ui \
> resources/ui/virt-viewer-file-transfer-dialog.ui \
> $(NULL)
>
> @@ -97,6 +98,8 @@ if HAVE_OVIRT
> libvirt_viewer_la_SOURCES += \
> ovirt-foreign-menu.h \
> ovirt-foreign-menu.c \
> + remote-viewer-iso-list-dialog.c \
> + remote-viewer-iso-list-dialog.h \
> $(NULL)
> endif
>
> diff --git a/src/remote-viewer-iso-list-dialog.c
> b/src/remote-viewer-iso-list-dialog.c
> new file mode 100644
> index 000..bf7c6c7
> --- /dev/null
> +++ b/src/remote-viewer-iso-list-dialog.c
> @@ -0,0 +1,365 @@
> +/*
> + * Virt Viewer: A virtual machine console viewer
> + *
> + * Copyright (C) 2016 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include
> +
> +#include
> +
> +#include "remote-viewer-iso-list-dialog.h"
> +#include "virt-viewer-util.h"
> +#include "ovirt-foreign-menu.h"
> +
> +static void ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu
> *foreign_menu, GAsyncResult *result, RemoteViewerISOListDialog *self);
> +static void
> remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self,
> const gchar *message);
> +
> +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog,
> GTK_TYPE_DIALOG)
> +
> +#define DIALOG_PRIVATE(o) \
> +(G_TYPE_INSTANCE_GET_PRIVATE((o),
> REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate))
> +
> +struct _RemoteViewerISOListDialogPrivate
> +{
> +GtkListStore *list_store;
> +GtkWidget *status;
> +GtkWidget *spinner;
> +GtkWidget *stack;
> +GtkWidget *tree_view;
> +OvirtForeignMenu *foreign_menu;
> +};
> +
> +enum RemoteViewerISOListDialogModel
> +{
> +ISO_IS_ACTIVE = 0,
> +ISO_NAME,
> +FONT_WEIGHT,
> +};
> +
> +enum RemoteViewerISOListDialogProperties {
> +PROP_0,
> +PROP_FOREIGN_MENU,
> +};
> +
> +
> +void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle
> *cell_renderer, gchar *path, gpointer user_data);
> +void remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view,
> GtkTreePath *path, GtkTreeViewColumn *col, gpointer user_data);
> +
> +static void
> +remote_viewer_iso_list_dialog_dispose(GObject *object)
> +{
> +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
> +RemoteViewerISOListDialogPrivate *priv = self->priv;
> +
> +if (priv->foreign_menu) {
> +g_signal_handlers_disconnect_by_data(priv->foreign_menu, object);
> +g_clear_object(&priv->foreign_m
[virt-tools-list] [PATCH virt-viewer 2/5] Introduce ISO List dialog
Signed-off-by: Eduardo Lima (Etrunko)
---
po/POTFILES.in | 2 +
src/Makefile.am| 3 +
src/remote-viewer-iso-list-dialog.c| 365 +
src/remote-viewer-iso-list-dialog.h| 58 +
src/resources/ui/remote-viewer-iso-list.ui | 158 +
src/resources/virt-viewer.gresource.xml| 1 +
6 files changed, 587 insertions(+)
create mode 100644 src/remote-viewer-iso-list-dialog.c
create mode 100644 src/remote-viewer-iso-list-dialog.h
create mode 100644 src/resources/ui/remote-viewer-iso-list.ui
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 69d9fef..371c242 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -1,9 +1,11 @@
data/remote-viewer.appdata.xml.in
data/remote-viewer.desktop.in
data/virt-viewer-mime.xml.in
+src/remote-viewer-iso-list-dialog.c
src/remote-viewer-main.c
src/remote-viewer.c
[type: gettext/glade] src/resources/ui/remote-viewer-connect.ui
+[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui
[type: gettext/glade] src/resources/ui/virt-viewer-about.ui
src/virt-viewer-app.c
src/virt-viewer-auth.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 272c4ff..9748277 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -13,6 +13,7 @@ noinst_DATA = \
resources/ui/virt-viewer-vm-connection.ui \
resources/ui/virt-viewer-preferences.ui \
resources/ui/remote-viewer-connect.ui \
+ resources/ui/remote-viewer-iso-list.ui \
resources/ui/virt-viewer-file-transfer-dialog.ui \
$(NULL)
@@ -97,6 +98,8 @@ if HAVE_OVIRT
libvirt_viewer_la_SOURCES += \
ovirt-foreign-menu.h \
ovirt-foreign-menu.c \
+ remote-viewer-iso-list-dialog.c \
+ remote-viewer-iso-list-dialog.h \
$(NULL)
endif
diff --git a/src/remote-viewer-iso-list-dialog.c
b/src/remote-viewer-iso-list-dialog.c
new file mode 100644
index 000..bf7c6c7
--- /dev/null
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -0,0 +1,365 @@
+/*
+ * Virt Viewer: A virtual machine console viewer
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include
+
+#include
+
+#include "remote-viewer-iso-list-dialog.h"
+#include "virt-viewer-util.h"
+#include "ovirt-foreign-menu.h"
+
+static void ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu
*foreign_menu, GAsyncResult *result, RemoteViewerISOListDialog *self);
+static void remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog
*self, const gchar *message);
+
+G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog,
GTK_TYPE_DIALOG)
+
+#define DIALOG_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
RemoteViewerISOListDialogPrivate))
+
+struct _RemoteViewerISOListDialogPrivate
+{
+GtkListStore *list_store;
+GtkWidget *status;
+GtkWidget *spinner;
+GtkWidget *stack;
+GtkWidget *tree_view;
+OvirtForeignMenu *foreign_menu;
+};
+
+enum RemoteViewerISOListDialogModel
+{
+ISO_IS_ACTIVE = 0,
+ISO_NAME,
+FONT_WEIGHT,
+};
+
+enum RemoteViewerISOListDialogProperties {
+PROP_0,
+PROP_FOREIGN_MENU,
+};
+
+
+void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle
*cell_renderer, gchar *path, gpointer user_data);
+void remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view,
GtkTreePath *path, GtkTreeViewColumn *col, gpointer user_data);
+
+static void
+remote_viewer_iso_list_dialog_dispose(GObject *object)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+if (priv->foreign_menu) {
+g_signal_handlers_disconnect_by_data(priv->foreign_menu, object);
+g_clear_object(&priv->foreign_menu);
+}
+
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
+}
+
+static void
+remote_viewer_iso_list_dialog_set_property(GObject *object, guint property_id,
+ const GValue *value, GParamSpec
*pspec)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+switch (property_id) {
+case PROP_FOREIGN_MENU:
+
