On Wed, 2015-06-10 at 14:42 -0500, Jonathon Jongsma wrote:
> I feel that the way you've split these patches up is a little bit
> unusual. The first commit adds the new function (with the same name as
> the old function), but does not build the new files. The second commit
> adds the files to the build (fortunately, there are no symbol conflicts
> because the old connect_dialog() function has static linkage), and then
> the third commit removes the old function.
>
> I like the fact that you were trying to split the changes into distinct
> commits, but I feel that the way they're currently split actually makes
> things a little bit harder to review and bisect. I'd almost prefer a
> single commit in this case.
I still agree with Pavel that changes in behavior should probably be
moved to a separate commit, however.
> I'm also not sure that this function really
> needs to be in a separate source file. But I don't feel strongly about
> that.
>
> Jonathon
>
>
> On Wed, 2015-06-10 at 14:47 +0200, Lukas Venhoda wrote:
> > Sometimes the connect dialog didn't get a parent.
> > It didn't make much sense for it to be a dialog.
> >
> > Changed the connect dialog to a window.
> > Moved the dialog code to its own module.
> > Moved the UI definiton into an XML.
> >
> > Changed response ID from ambiguous -1 and 0 to GTK_RESPONSE_ACCEPT
> > and GTK_RESPONSE_REJECT.
> >
> > Address is now required to connect with enter, or connect button.
> > Also sets connect button to non-sensitive, if no text is in adress entry.
> >
> > Focusing in entries will now unselect recent connection.
> > This fixes an issue, when selecting a recent connection, modifying
> > the adress entry, and then doubleclicking the selected connection,
> > remote-viewer would try to connect to the address in address entry
> > instead of the recent connection.
> > Doubleclicking teh recent connection will also set address before
> > activating, just to be sure.
> > ---
> > src/remote-viewer-connect.c | 277 ++++++++++++++++++++++++++++++++++++++++++
> > src/remote-viewer-connect.h | 36 ++++++
> > src/remote-viewer-connect.xml | 152 +++++++++++++++++++++++
> > 3 files changed, 465 insertions(+)
> > create mode 100644 src/remote-viewer-connect.c
> > create mode 100644 src/remote-viewer-connect.h
> > create mode 100644 src/remote-viewer-connect.xml
> >
> > diff --git a/src/remote-viewer-connect.c b/src/remote-viewer-connect.c
> > new file mode 100644
> > index 0000000..6a0091e
> > --- /dev/null
> > +++ b/src/remote-viewer-connect.c
> > @@ -0,0 +1,277 @@
> > +/*
> > + * Virt Viewer: A virtual machine console viewer
> > + *
> > + * Copyright (C) 2015 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 "remote-viewer-connect.h"
> > +#include "virt-viewer-util.h"
> > +#include <glib/gi18n.h>
> > +#include <gdk/gdkkeysyms.h>
> > +
> > +typedef struct
> > +{
> > + GtkResponseType response_id;
> > + GMainLoop *loop;
> > + GtkEntry *entry;
> > +} ConnectionInfo;
> > +
> > +static void
> > +shutdown_loop(GMainLoop *loop)
> > +{
> > + if (g_main_loop_is_running(loop))
> > + g_main_loop_quit(loop);
> > +}
> > +
> > +static gboolean
> > +window_deleted_cb(GtkWindow *window G_GNUC_UNUSED,
> > + GdkEventAny *event G_GNUC_UNUSED,
> > + gpointer data)
> > +{
> > + ConnectionInfo *ci = data;
> > + ci->response_id = GTK_RESPONSE_REJECT;
> > + shutdown_loop(ci->loop);
> > + return TRUE;
> > +}
> > +
> > +static void
> > +cancel_button_clicked_cb(GtkButton *button G_GNUC_UNUSED,
> > + gpointer data)
> > +{
> > + ConnectionInfo *ci = data;
> > + ci->response_id = GTK_RESPONSE_REJECT;
> > + shutdown_loop(ci->loop);
> > +}
> > +
> > +static void
> > +connect_button_clicked_cb(GtkButton *button G_GNUC_UNUSED,
> > + gpointer data)
> > +{
> > + ConnectionInfo *ci = data;
> > + if (gtk_entry_get_text_length(GTK_ENTRY(ci->entry)) > 0)
> > + {
> > + ci->response_id = GTK_RESPONSE_ACCEPT;
> > + shutdown_loop(ci->loop);
> > + }
> > +}
> > +
> > +static void
> > +entry_icon_release_cb(GtkEntry* entry,
> > + gpointer data G_GNUC_UNUSED)
> > +{
> > + gtk_entry_set_text(entry, "");
> > + gtk_widget_grab_focus(GTK_WIDGET(entry));
> > +}
> > +
> > +static void
> > +entry_changed_cb(GtkEditable* entry,
> > + gpointer data)
> > +{
> > + GtkButton *connect_button = data;
> > + gboolean rtl = (gtk_widget_get_direction(GTK_WIDGET(entry)) == GTK_TEXT_DIR_RTL);
> > + gboolean active = (gtk_entry_get_text_length(GTK_ENTRY(entry)) > 0);
> > +
> > + gtk_widget_set_sensitive(GTK_WIDGET(connect_button), active);
> > +
> > + g_object_set(entry,
> > + "secondary-icon-name", active ? (rtl ? "edit-clear-rtl-symbolic" : "edit-clear-symbolic") : NULL,
> > + "secondary-icon-activatable", active,
> > + "secondary-icon-sensitive", active,
> > + NULL);
> > +}
> > +
> > +static gboolean
> > +entry_focus_in_cb(GtkWidget *widget G_GNUC_UNUSED,
> > + GdkEvent *event G_GNUC_UNUSED,
> > + gpointer data)
> > +{
> > + GtkRecentChooser *recent = data;
> > + gtk_recent_chooser_unselect_all(recent);
> > + return TRUE;
> > +}
> > +
> > +static void
> > +entry_activated_cb(GtkEntry *entry G_GNUC_UNUSED,
> > + gpointer data)
> > +{
> > + ConnectionInfo *ci = data;
> > + if (gtk_entry_get_text_length(GTK_ENTRY(ci->entry)) > 0)
> > + {
> > + ci->response_id = GTK_RESPONSE_ACCEPT;
> > + shutdown_loop(ci->loop);
> > + }
> > +}
> > +
> > +static void
> > +recent_item_activated_dialog_cb(GtkRecentChooser *chooser G_GNUC_UNUSED,
> > + gpointer data)
> > +{
> > + ConnectionInfo *ci = data;
> > + GtkRecentInfo *info;
> > + const gchar *uri;
> > +
> > + info = gtk_recent_chooser_get_current_item(chooser);
> > + if (info == NULL)
> > + return;
> > +
> > + uri = gtk_recent_info_get_uri(info);
> > + g_return_if_fail(uri != NULL);
> > +
> > + gtk_entry_set_text(ci->entry, uri);
> > +
> > + gtk_recent_info_unref(info);
> > +
> > + ci->response_id = GTK_RESPONSE_ACCEPT;
> > + shutdown_loop(ci->loop);
> > +}
> > +
> > +static void
> > +recent_selection_changed_dialog_cb(GtkRecentChooser *chooser,
> > + gpointer data)
> > +{
> > + GtkRecentInfo *info;
> > + const gchar *uri;
> > + ConnectionInfo *ci = data;
> > +
> > + info = gtk_recent_chooser_get_current_item(chooser);
> > + if (info == NULL)
> > + return;
> > +
> > + uri = gtk_recent_info_get_uri(info);
> > + g_return_if_fail(uri != NULL);
> > +
> > + gtk_entry_set_text(ci->entry, uri);
> > +
> > + gtk_recent_info_unref(info);
> > +}
> > +
> > +static gboolean
> > +key_pressed_cb(GtkWidget *widget G_GNUC_UNUSED,
> > + GdkEvent *event,
> > + gpointer data)
> > +{
> > + GtkWidget *window = data;
> > + gboolean retval;
> > + if (event->type == GDK_KEY_PRESS) {
> > + switch (event->key.keyval) {
> > + case GDK_KEY_Escape:
> > + g_signal_emit_by_name(window, "delete-event", NULL, &retval);
> > + return TRUE;
> > + default:
> > + return FALSE;
> > + }
> > + }
> > +
> > + return FALSE;
> > +}
> > +
> > +static void
> > +make_label_smaller(GtkLabel* label)
> > +{
> > + PangoAttrList *attributes;
> > +
> > + attributes = pango_attr_list_new();
> > + pango_attr_list_insert(attributes, pango_attr_scale_new(0.9));
> > + gtk_label_set_attributes(label, attributes);
> > + pango_attr_list_unref(attributes);
> > +}
> > +
> > +gint
> > +connect_dialog(gchar **uri G_GNUC_UNUSED)
> > +{
> > + GtkWidget *window, *connect_button, *cancel_button, *recent, *example_label;
> > + GtkRecentFilter *rfilter;
> > + GtkBuilder *builder;
> > + gboolean active;
> > +
> > + ConnectionInfo ci = {
> > + GTK_RESPONSE_NONE,
> > + NULL,
> > + NULL
> > + };
> > +
> > + builder = virt_viewer_util_load_ui("remote-viewer-connect.xml");
> > + g_return_val_if_fail(builder != NULL, NULL);
> > +
> > + window = GTK_WIDGET(gtk_builder_get_object(builder, "remote-viewer-connection-window"));
> > + connect_button = GTK_WIDGET(gtk_builder_get_object(builder, "connect-button"));
> > + cancel_button = GTK_WIDGET(gtk_builder_get_object(builder, "cancel-button"));
> > + example_label = GTK_WIDGET(gtk_builder_get_object(builder, "example-label"));
> > + ci.entry = GTK_ENTRY(gtk_builder_get_object(builder, "connection-address-entry"));
> > +
> > + make_label_smaller(GTK_LABEL(example_label));
> > +
> > + active = (gtk_entry_get_text_length(GTK_ENTRY(ci.entry)) > 0);
> > + gtk_widget_set_sensitive(GTK_WIDGET(connect_button), active);
> > +
> > + recent = GTK_WIDGET(gtk_builder_get_object(builder, "recent-chooser"));
> > +
> > + rfilter = gtk_recent_filter_new();
> > + gtk_recent_filter_add_mime_type(rfilter, "application/x-spice");
> > + gtk_recent_filter_add_mime_type(rfilter, "application/x-vnc");
> > + gtk_recent_filter_add_mime_type(rfilter, "application/x-virt-viewer");
> > + gtk_recent_chooser_set_filter(GTK_RECENT_CHOOSER(recent), rfilter);
> > + gtk_recent_chooser_set_local_only(GTK_RECENT_CHOOSER(recent), FALSE);
> > +
> > + g_signal_connect(window, "delete-event",
> > + G_CALLBACK(window_deleted_cb), &ci);
> > + g_signal_connect(window, "key-press-event",
> > + G_CALLBACK(key_pressed_cb), window);
> > + g_signal_connect(connect_button, "clicked",
> > + G_CALLBACK(connect_button_clicked_cb), &ci);
> > + g_signal_connect(cancel_button, "clicked",
> > + G_CALLBACK(cancel_button_clicked_cb), &ci);
> > +
> > + g_signal_connect(ci.entry, "activate",
> > + G_CALLBACK(entry_activated_cb), &ci);
> > + g_signal_connect(ci.entry, "changed",
> > + G_CALLBACK(entry_changed_cb), connect_button);
> > + g_signal_connect(ci.entry, "icon-release",
> > + G_CALLBACK(entry_icon_release_cb), ci.entry);
> > + g_signal_connect(ci.entry, "focus-in-event",
> > + G_CALLBACK(entry_focus_in_cb), recent);
> > +
> > + g_signal_connect(recent, "selection-changed",
> > + G_CALLBACK(recent_selection_changed_dialog_cb), &ci);
> > + g_signal_connect(recent, "item-activated",
> > + G_CALLBACK(recent_item_activated_dialog_cb), &ci);
> > +
> > + gtk_widget_show_all(window);
> > +
> > + ci.loop = g_main_loop_new(NULL, FALSE);
> > + g_main_loop_run(ci.loop);
> > +
> > + if (ci.response_id == GTK_RESPONSE_ACCEPT) {
> > + *uri = g_strdup(gtk_entry_get_text(GTK_ENTRY(ci.entry)));
> > + g_strstrip(*uri);
> > + } else {
> > + *uri = NULL;
> > + }
> > +
> > + g_object_unref(builder);
> > + gtk_widget_destroy(window);
> > +
> > + return ci.response_id;
> > +}
> > +
> > +/*
> > + * Local variables:
> > + * c-indent-level: 4
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > diff --git a/src/remote-viewer-connect.h b/src/remote-viewer-connect.h
> > new file mode 100644
> > index 0000000..cc4776b
> > --- /dev/null
> > +++ b/src/remote-viewer-connect.h
> > @@ -0,0 +1,36 @@
> > +/*
> > + * Virt Viewer: A virtual machine console viewer
> > + *
> > + * Copyright (C) 2015 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
> > + */
> > +
> > +#ifndef REMOTE_VIEWER_CONNECT_H
> > +#define REMOTE_VIEWER_CONNECT_H
> > +
> > +#include <gtk/gtk.h>
> > +
> > +gint connect_dialog(gchar **uri);
> > +
> > +#endif /* REMOTE_VIEWER_CONNECT_H */
> > +
> > +/*
> > + * Local variables:
> > + * c-indent-level: 4
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > diff --git a/src/remote-viewer-connect.xml b/src/remote-viewer-connect.xml
> > new file mode 100644
> > index 0000000..dcd14cf
> > --- /dev/null
> > +++ b/src/remote-viewer-connect.xml
> > @@ -0,0 +1,152 @@
> > +<?xml version="1.0" encoding="UTF-8"?>
> > +<!-- Generated with glade 3.18.3 -->
> > +<interface>
> > + <object class="GtkWindow" id="remote-viewer-connection-window">
> > + <property name="can_focus">False</property>
> > + <property name="title" translatable="yes">Connection details</property>
> > + <child>
> > + <object class="GtkVBox" id="main-box">
> > + <property name="visible">True</property>
> > + <property name="can_focus">False</property>
> > + <property name="border_width">10</property>
> > + <property name="spacing">20</property>
> > + <child>
> > + <object class="GtkVBox" id="connection-address-box">
> > + <property name="visible">True</property>
> > + <property name="can_focus">False</property>
> > + <property name="spacing">6</property>
> > + <child>
> > + <object class="GtkLabel" id="connection-address-label">
> > + <property name="visible">True</property>
> > + <property name="can_focus">False</property>
> > + <property name="label" translatable="yes">Connection Address</property>
> > + <property name="xalign">0</property>
> > + <attributes>
> > + <attribute name="weight" value="bold"/>
> > + </attributes>
> > + </object>
> > + <packing>
> > + <property name="expand">False</property>
> > + <property name="fill">True</property>
> > + <property name="position">0</property>
> > + </packing>
> > + </child>
> > + <child>
> > + <object class="GtkEntry" id="connection-address-entry">
> > + <property name="visible">True</property>
> > + <property name="can_focus">True</property>
> > + </object>
> > + <packing>
> > + <property name="expand">False</property>
> > + <property name="fill">True</property>
> > + <property name="position">1</property>
> > + </packing>
> > + </child>
> > + <child>
> > + <object class="GtkLabel" id="example-label">
> > + <property name="visible">True</property>
> > + <property name="can_focus">False</property>
> > + <property name="xalign">0</property>
> > + <property name="sensitive">False</property>
> > + <property name="label" translatable="yes">For example, spice://foo.example.org:5900</property>
> > + </object>
> > + <packing>
> > + <property name="expand">False</property>
> > + <property name="fill">True</property>
> > + <property name="position">2</property>
> > + </packing>
> > + </child>
> > + </object>
> > + <packing>
> > + <property name="expand">False</property>
> > + <property name="fill">True</property>
> > + <property name="position">0</property>
> > + </packing>
> > + </child>
> > + <child>
> > + <object class="GtkVBox" id="recent-chooser-box">
> > + <property name="visible">True</property>
> > + <property name="can_focus">False</property>
> > + <property name="spacing">6</property>
> > + <child>
> > + <object class="GtkLabel" id="recent-chooser-label">
> > + <property name="visible">True</property>
> > + <property name="can_focus">False</property>
> > + <property name="label" translatable="yes">Recent connections</property>
> > + <property name="xalign">0</property>
> > + <attributes>
> > + <attribute name="weight" value="bold"/>
> > + </attributes>
> > + </object>
> > + <packing>
> > + <property name="expand">False</property>
> > + <property name="fill">True</property>
> > + <property name="position">0</property>
> > + </packing>
> > + </child>
> > + <child>
> > + <object class="GtkRecentChooserWidget" id="recent-chooser">
> > + <property name="can_focus">False</property>
> > + <property name="limit">20</property>
> > + <property name="local_only">False</property>
> > + <property name="show_icons">False</property>
> > + <property name="sort_type">mru</property>
> > + </object>
> > + <packing>
> > + <property name="expand">True</property>
> > + <property name="fill">True</property>
> > + <property name="position">1</property>
> > + </packing>
> > + </child>
> > + </object>
> > + <packing>
> > + <property name="expand">True</property>
> > + <property name="fill">True</property>
> > + <property name="position">2</property>
> > + </packing>
> > + </child>
> > + <child>
> > + <object class="GtkHButtonBox" id="button-box">
> > + <property name="visible">True</property>
> > + <property name="can_focus">False</property>
> > + <property name="resize_mode">immediate</property>
> > + <property name="spacing">6</property>
> > + <property name="layout_style">end</property>
> > + <child>
> > + <object class="GtkButton" id="cancel-button">
> > + <property name="label" translatable="yes">Cancel</property>
> > + <property name="visible">True</property>
> > + <property name="can_focus">True</property>
> > + <property name="receives_default">True</property>
> > + </object>
> > + <packing>
> > + <property name="expand">True</property>
> > + <property name="fill">True</property>
> > + <property name="position">0</property>
> > + </packing>
> > + </child>
> > + <child>
> > + <object class="GtkButton" id="connect-button">
> > + <property name="label" translatable="yes">Connect</property>
> > + <property name="visible">True</property>
> > + <property name="can_focus">True</property>
> > + <property name="receives_default">True</property>
> > + </object>
> > + <packing>
> > + <property name="expand">True</property>
> > + <property name="fill">True</property>
> > + <property name="position">1</property>
> > + </packing>
> > + </child>
> > + </object>
> > + <packing>
> > + <property name="expand">False</property>
> > + <property name="fill">False</property>
> > + <property name="pack_type">end</property>
> > + <property name="position">3</property>
> > + </packing>
> > + </child>
> > + </object>
> > + </child>
> > + </object>
> > +</interface>
> > --
> > 2.4.2
> >
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
>
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/virt-tools-list
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
