[virt-tools-list] [PATCH virt-viewer 2/5] Introduce ISO List dialog
Christophe Fergeau
cfergeau at redhat.com
Fri Jan 20 15:20:20 UTC 2017
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) <etrunko at redhat.com>
> ---
> 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 0000000..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 <config.h>
> +
> +#include <glib/gi18n.h>
> +
> +#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:
> + priv->foreign_menu = g_value_dup_object(value);
> + break;
> + default:
> + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
> + }
> +}
> +
> +static void
> +remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
> +{
> + GObjectClass *object_class = G_OBJECT_CLASS(klass);
> +
> + g_type_class_add_private(klass, sizeof(RemoteViewerISOListDialogPrivate));
> +
> + object_class->dispose = remote_viewer_iso_list_dialog_dispose;
> + object_class->set_property = remote_viewer_iso_list_dialog_set_property;
> +
> + g_object_class_install_property(object_class,
> + PROP_FOREIGN_MENU,
> + g_param_spec_object("foreign-menu",
> + "oVirt Foreign Menu",
> + "Object which is used as interface to oVirt",
> + OVIRT_TYPE_FOREIGN_MENU,
> + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS));
s/G_PARAM_READWRITE/G_PARAM_WRITABLE
> +}
> +
> +static void
> +remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self)
> +{
> + RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
I prefer to avoid multiple assignments on a single line. Here you don't
really need a local 'priv' anyway, so this could just be
self->priv = DIALOG_PRIVATE(self);
> + gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "iso-list",
> + GTK_STACK_TRANSITION_TYPE_NONE);
> + gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE);
> +}
> +
> +static void
> +remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *self)
> +{
> + RemoteViewerISOListDialogPrivate *priv = self->priv;
> + gchar *current_iso = ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu);
> + gboolean active = g_strcmp0(current_iso, name) == 0;
I'd prefer
gboolean active = (g_strcmp0(current_iso, name) == 0);
imo this makes it easier to notice we're assigning the result of a test
to the boolean.
> + gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
> + GtkTreeIter iter;
> +
> + gtk_list_store_append(priv->list_store, &iter);
> + gtk_list_store_set(priv->list_store, &iter,
> + ISO_IS_ACTIVE, active,
> + ISO_NAME, name,
> + FONT_WEIGHT, weight, -1);
> +
> + if (active) {
> + GtkTreePath *path = gtk_tree_model_get_path(GTK_TREE_MODEL(priv->list_store), &iter);
> + gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), path, NULL, FALSE);
> + gtk_tree_view_scroll_to_cell(GTK_TREE_VIEW(priv->tree_view), path, NULL, TRUE, 0.5, 0.5);
> + gtk_tree_path_free(path);
> + }
> +
> + g_free(current_iso);
> +}
> +
> +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("<b>%s</b>", 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).
> + return;
> + }
> +
> + g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self);
> + remote_viewer_iso_list_dialog_show_files(self);
> +}
> +
> +
> +static void
> +remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self)
> +{
> + RemoteViewerISOListDialogPrivate *priv = self->priv;
> +
> + gtk_list_store_clear(priv->list_store);
> + ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL,
> + (GAsyncReadyCallback) fetch_iso_names_cb,
> + self);
> +}
> +
> +static void
> +remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
> + gint response_id,
> + gpointer user_data G_GNUC_UNUSED)
> +{
> + RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
> + RemoteViewerISOListDialogPrivate *priv = self->priv;
> +
> + if (response_id != GTK_RESPONSE_NONE)
> + return;
> +
> + gtk_spinner_start(GTK_SPINNER(priv->spinner));
> + gtk_label_set_markup(GTK_LABEL(priv->status), _("<b>Loading...</b>"));
> + gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
> + GTK_STACK_TRANSITION_TYPE_NONE);
> + gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE);
> + remote_viewer_iso_list_dialog_refresh_iso_list(self);
> +}
> +
> +void
> +remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNUC_UNUSED,
> + gchar *path,
> + gpointer user_data)
> +{
> + RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data);
> + RemoteViewerISOListDialogPrivate *priv = self->priv;
> + GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store);
> + GtkTreePath *tree_path = gtk_tree_path_new_from_string(path);
> + GtkTreeIter iter;
> + gboolean active;
> + gchar *name;
> +
> + gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), tree_path, NULL, FALSE);
> + gtk_tree_model_get_iter(model, &iter, tree_path);
> + gtk_tree_model_get(model, &iter,
> + ISO_IS_ACTIVE, &active,
> + ISO_NAME, &name, -1);
> +
> + gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE);
> + gtk_widget_set_sensitive(priv->tree_view, FALSE);
> +
> + ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, NULL,
> + (GAsyncReadyCallback)ovirt_foreign_menu_iso_name_changed,
> + self);
> + gtk_tree_path_free(tree_path);
> + g_free(name);
> +}
> +
> +void
> +remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view G_GNUC_UNUSED,
> + GtkTreePath *path,
> + GtkTreeViewColumn *col G_GNUC_UNUSED,
> + gpointer user_data)
> +{
> + gchar *path_str = gtk_tree_path_to_string(path);
> + remote_viewer_iso_list_dialog_toggled(NULL, path_str, user_data);
> + g_free(path_str);
> +}
> +
> +static void
> +remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
> +{
> + GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self));
> + RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
> + GtkBuilder *builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui");
> + GtkCellRendererToggle *cell_renderer;
> +
> + gtk_builder_connect_signals(builder, self);
> +
> + priv->status = GTK_WIDGET(gtk_builder_get_object(builder, "status"));
> + priv->spinner = GTK_WIDGET(gtk_builder_get_object(builder, "spinner"));
> + priv->stack = GTK_WIDGET(gtk_builder_get_object(builder, "stack"));
> + gtk_box_pack_start(GTK_BOX(content), priv->stack, TRUE, TRUE, 0);
> +
> + priv->list_store = GTK_LIST_STORE(gtk_builder_get_object(builder, "liststore"));
> + priv->tree_view = GTK_WIDGET(gtk_builder_get_object(builder, "view"));
> + cell_renderer = GTK_CELL_RENDERER_TOGGLE(gtk_builder_get_object(builder, "cellrenderertoggle"));
> + gtk_cell_renderer_toggle_set_radio(cell_renderer, TRUE);
> + gtk_cell_renderer_set_padding(GTK_CELL_RENDERER(cell_renderer), 6, 6);
> +
> + g_object_unref(builder);
> +
> + gtk_dialog_add_buttons(GTK_DIALOG(self),
> + _("Refresh"), GTK_RESPONSE_NONE,
> + _("Close"), GTK_RESPONSE_CLOSE,
> + NULL);
> +
> + gtk_dialog_set_default_response(GTK_DIALOG(self), GTK_RESPONSE_CLOSE);
> + gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE);
> + g_signal_connect(self, "response", G_CALLBACK(remote_viewer_iso_list_dialog_response), NULL);
> +}
> +
> +static void
> +remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self,
> + const gchar *message)
> +{
> + GtkWidget *dialog;
> +
> + g_warn_if_fail(message != NULL);
> +
> + dialog = gtk_message_dialog_new(GTK_WINDOW(self),
> + GTK_DIALOG_DESTROY_WITH_PARENT,
> + GTK_MESSAGE_ERROR,
> + GTK_BUTTONS_CLOSE,
> + message ? message : _("Unspecified error"));
> + gtk_dialog_run(GTK_DIALOG(dialog));
> + gtk_widget_destroy(dialog);
> +}
> +
> +static void
> +ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu,
> + GAsyncResult *result,
> + RemoteViewerISOListDialog *self)
> +{
> + RemoteViewerISOListDialogPrivate *priv = self->priv;
> + GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store);
> + gchar *current_iso;
> + GtkTreeIter iter;
> + gchar *name;
> + gboolean active, match = FALSE;
> + GError *error = NULL;
> +
> + /* In the case of error, don't return early, because it is necessary to
> + * change the ISO back to the original, avoiding a possible inconsistency.
> + */
> + if (!ovirt_foreign_menu_set_current_iso_name_finish(foreign_menu, result, &error)) {
> + remote_viewer_iso_list_dialog_show_error(self, error ? error->message : _("Failed to change CD"));
> + g_clear_error(&error);
> + }
> +
> + if (!gtk_tree_model_get_iter_first(model, &iter))
> + return;
> +
> + current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
> +
> + do {
> + gtk_tree_model_get(model, &iter,
> + ISO_IS_ACTIVE, &active,
> + ISO_NAME, &name, -1);
> + match = g_strcmp0(current_iso, name) == 0;
match = (g_strcmp0(current_iso, name) == 0);
Reviewed-by: Christophe Fergeau <cfergeau at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170120/85837881/attachment.sig>
More information about the virt-tools-list
mailing list