[virt-tools-list] [remote-viewer PATCH 1/7 v2] remote-viewer: Connect dialog moved to its own file

Lukas Venhoda lvenhoda at redhat.com
Thu Jun 11 17:00:06 UTC 2015


Hi,

>I see that you're just following the example of the previous line, but I
>prefer to have a single file per line.

Should I also change the previous line?
As commit No. 8 perhaps?

Lukas

----- Original Message -----
From: "Jonathon Jongsma" <jjongsma at redhat.com>
To: "Lukas Venhoda" <lvenhoda at redhat.com>
Cc: virt-tools-list at redhat.com
Sent: Thursday, June 11, 2015 6:52:37 PM
Subject: Re: [virt-tools-list] [remote-viewer PATCH 1/7 v2] remote-viewer: Connect dialog moved to its own file

On Thu, 2015-06-11 at 16:28 +0200, Lukas Venhoda wrote:
> Connect dialog from remote-viewer is now in its own file.
> Most other dialog also have their own files.
> This will make changing the dialog into a window easier.
> ---
> Changes since v1
>  - New patch
>  - Reversed order of patches. Now create a moduel first, modify after.
>  - Renamed the function from connect_dialog to remote_viewer_connect_dialog

Perhaps also mention this rename in the commit message above

> ---
>  src/Makefile.am             |   1 +
>  src/remote-viewer-connect.c | 189 ++++++++++++++++++++++++++++++++++++++++++++
>  src/remote-viewer-connect.h |  36 +++++++++
>  src/remote-viewer.c         | 160 +------------------------------------
>  4 files changed, 228 insertions(+), 158 deletions(-)
>  create mode 100644 src/remote-viewer-connect.c
>  create mode 100644 src/remote-viewer-connect.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index c06ba85..2427abb 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -134,6 +134,7 @@ bin_PROGRAMS += remote-viewer
>  remote_viewer_SOURCES =				\
>  	$(COMMON_SOURCES)			\
>  	remote-viewer.h remote-viewer.c		\
> +	remote-viewer-connect.h remote-viewer-connect.c \

I see that you're just following the example of the previous line, but I
prefer to have a single file per line.

>  	remote-viewer-main.c			\
>  	$(NULL)
>  remote_viewer_LDFLAGS =				\
> diff --git a/src/remote-viewer-connect.c b/src/remote-viewer-connect.c
> new file mode 100644
> index 0000000..a459f6a
> --- /dev/null
> +++ b/src/remote-viewer-connect.c
> @@ -0,0 +1,189 @@
> +/*
> + * 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 <glib/gi18n.h>
> +
> +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 G_GNUC_UNUSED)
> +{
> +    gboolean rtl = (gtk_widget_get_direction(GTK_WIDGET(entry)) == GTK_TEXT_DIR_RTL);
> +    gboolean active = (gtk_entry_get_text_length(GTK_ENTRY(entry)) > 0);
> +
> +    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 void
> +recent_selection_changed_dialog_cb(GtkRecentChooser *chooser, gpointer data)
> +{
> +    GtkRecentInfo *info;
> +    GtkWidget *entry = data;
> +    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(GTK_ENTRY(entry), uri);
> +
> +    gtk_recent_info_unref(info);
> +}
> +
> +static void
> +recent_item_activated_dialog_cb(GtkRecentChooser *chooser G_GNUC_UNUSED, gpointer data)
> +{
> +   gtk_dialog_response(GTK_DIALOG (data), GTK_RESPONSE_ACCEPT);
> +}
> +
> +static void
> +make_label_light(GtkLabel* label)
> +{
> +    PangoAttrList* attributes = pango_attr_list_new();
> +#if GTK_CHECK_VERSION(3, 0, 0)
> +    gtk_style_context_add_class(gtk_widget_get_style_context(GTK_WIDGET(label)), "dim-label");
> +#else
> +    GtkStyle* style = gtk_widget_get_style(GTK_WIDGET(label));
> +    GdkColor* c = &style->text[GTK_STATE_INSENSITIVE];
> +    pango_attr_list_insert(attributes, pango_attr_foreground_new(c->red, c->green, c->blue));
> +#endif
> +    pango_attr_list_insert(attributes, pango_attr_scale_new(0.9));
> +    gtk_label_set_attributes(label, attributes);
> +    pango_attr_list_unref(attributes);
> +}
> +
> +static void
> +make_label_bold(GtkLabel* label)
> +{
> +    PangoAttrList* attributes = pango_attr_list_new();
> +    pango_attr_list_insert(attributes, pango_attr_weight_new(PANGO_WEIGHT_BOLD));
> +    gtk_label_set_attributes(label, attributes);
> +    pango_attr_list_unref(attributes);
> +}
> +
> +gint
> +remote_viewer_connect_dialog(GtkWindow *main_window, gchar **uri)
> +{
> +    GtkWidget *dialog, *area, *box, *label, *entry, *recent;
> +#if !GTK_CHECK_VERSION(3, 0, 0)
> +    GtkWidget *alignment;
> +#endif
> +    GtkRecentFilter *rfilter;
> +    gint retval;
> +
> +    /* Create the widgets */
> +    dialog = gtk_dialog_new_with_buttons(_("Connection details"),
> +                                         main_window,
> +                                         GTK_DIALOG_DESTROY_WITH_PARENT,
> +                                         GTK_STOCK_CANCEL,
> +                                         GTK_RESPONSE_REJECT,
> +                                         GTK_STOCK_CONNECT,
> +                                         GTK_RESPONSE_ACCEPT,
> +                                         NULL);
> +    gtk_dialog_set_default_response(GTK_DIALOG(dialog), GTK_RESPONSE_ACCEPT);
> +    gtk_container_set_border_width(GTK_CONTAINER(dialog), 5);
> +    area = gtk_dialog_get_content_area(GTK_DIALOG(dialog));
> +    box = gtk_vbox_new(FALSE, 6);
> +    gtk_container_set_border_width(GTK_CONTAINER(box), 5);
> +    gtk_box_pack_start(GTK_BOX(area), box, TRUE, TRUE, 0);
> +
> +    label = gtk_label_new_with_mnemonic(_("_Connection Address"));
> +    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> +    gtk_box_pack_start(GTK_BOX(box), label, TRUE, TRUE, 0);
> +    entry = GTK_WIDGET(gtk_entry_new());
> +    gtk_entry_set_activates_default(GTK_ENTRY(entry), TRUE);
> +    g_object_set(entry, "width-request", 200, NULL);
> +    g_signal_connect(entry, "changed", G_CALLBACK(entry_changed_cb), entry);
> +    g_signal_connect(entry, "icon-release", G_CALLBACK(entry_icon_release_cb), entry);
> +    gtk_box_pack_start(GTK_BOX(box), entry, TRUE, TRUE, 0);
> +    gtk_label_set_mnemonic_widget(GTK_LABEL(label), entry);
> +    make_label_bold(GTK_LABEL(label));
> +
> +    label = gtk_label_new(_("For example, spice://foo.example.org:5900"));
> +    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> +    make_label_light(GTK_LABEL(label));
> +#if GTK_CHECK_VERSION(3, 0, 0)
> +    gtk_box_pack_start(GTK_BOX(box), label, TRUE, TRUE, 0);
> +    gtk_widget_set_margin_bottom(label, 12);
> +#else
> +    alignment = gtk_alignment_new(0, 0, 1, 1);
> +    gtk_alignment_set_padding(GTK_ALIGNMENT(alignment), 0, 12, 0, 0);
> +    gtk_container_add(GTK_CONTAINER(alignment), label);
> +    gtk_box_pack_start(GTK_BOX(box), alignment, TRUE, TRUE, 0);
> +#endif
> +
> +    label = gtk_label_new_with_mnemonic(_("_Recent Connections"));
> +    make_label_bold(GTK_LABEL(label));
> +    gtk_box_pack_start(GTK_BOX(box), label, TRUE, TRUE, 0);
> +    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> +
> +    recent = GTK_WIDGET(gtk_recent_chooser_widget_new());
> +    gtk_recent_chooser_set_show_icons(GTK_RECENT_CHOOSER(recent), FALSE);
> +    gtk_recent_chooser_set_sort_type(GTK_RECENT_CHOOSER(recent), GTK_RECENT_SORT_MRU);
> +    gtk_box_pack_start(GTK_BOX(box), recent, TRUE, TRUE, 0);
> +    gtk_label_set_mnemonic_widget(GTK_LABEL(label), recent);
> +
> +    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(recent, "selection-changed",
> +                     G_CALLBACK(recent_selection_changed_dialog_cb), entry);
> +    g_signal_connect(recent, "item-activated",
> +                     G_CALLBACK(recent_item_activated_dialog_cb), dialog);
> +
> +    /* show and wait for response */
> +    gtk_widget_show_all(dialog);
> +
> +    if (gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
> +        *uri = g_strdup(gtk_entry_get_text(GTK_ENTRY(entry)));
> +        g_strstrip(*uri);
> +        retval = 0;
> +    } else {
> +        *uri = NULL;
> +        retval = -1;
> +    }
> +    gtk_widget_destroy(dialog);
> +
> +    return retval;
> +}
> +
> +/*
> + * 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..86705d7
> --- /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 remote_viewer_connect_dialog(GtkWindow *main_window, 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.c b/src/remote-viewer.c
> index 937d06d..76b12ae 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -46,6 +46,7 @@
>  #include "virt-viewer-file.h"
>  #include "virt-viewer-session.h"
>  #include "remote-viewer.h"
> +#include "remote-viewer-connect.h"
> 
>  #ifndef G_VALUE_INIT /* see bug https://bugzilla.gnome.org/show_bug.cgi?id=654793 */
>  #define G_VALUE_INIT  { 0, { { 0 } } }
> @@ -87,7 +88,6 @@ static gboolean remote_viewer_start(VirtViewerApp *self, GError **error);
>  static gboolean remote_viewer_activate(VirtViewerApp *self, GError **error);
>  static void remote_viewer_window_added(VirtViewerApp *self, VirtViewerWindow *win);
>  static void spice_foreign_menu_updated(RemoteViewer *self);
> -static gint connect_dialog(GtkWindow *main_window, gchar **uri);
>  #endif
> 
>  static void
> @@ -1016,162 +1016,6 @@ error:
> 
>  #endif
> 
> -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 G_GNUC_UNUSED)
> -{
> -    gboolean rtl = (gtk_widget_get_direction(GTK_WIDGET(entry)) == GTK_TEXT_DIR_RTL);
> -    gboolean active = gtk_entry_get_text_length(GTK_ENTRY(entry)) > 0;
> -
> -    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 void
> -recent_selection_changed_dialog_cb(GtkRecentChooser *chooser, gpointer data)
> -{
> -    GtkRecentInfo *info;
> -    GtkWidget *entry = data;
> -    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(GTK_ENTRY(entry), uri);
> -
> -    gtk_recent_info_unref(info);
> -}
> -
> -static void
> -recent_item_activated_dialog_cb(GtkRecentChooser *chooser G_GNUC_UNUSED, gpointer data)
> -{
> -   gtk_dialog_response(GTK_DIALOG (data), GTK_RESPONSE_ACCEPT);
> -}
> -
> -static void make_label_light(GtkLabel* label)
> -{
> -    PangoAttrList* attributes = pango_attr_list_new();
> -#if GTK_CHECK_VERSION(3, 0, 0)
> -    gtk_style_context_add_class(gtk_widget_get_style_context(GTK_WIDGET(label)), "dim-label");
> -#else
> -    GtkStyle* style = gtk_widget_get_style(GTK_WIDGET(label));
> -    GdkColor* c = &style->text[GTK_STATE_INSENSITIVE];
> -    pango_attr_list_insert(attributes, pango_attr_foreground_new(c->red, c->green, c->blue));
> -#endif
> -    pango_attr_list_insert(attributes, pango_attr_scale_new(0.9));
> -    gtk_label_set_attributes(label, attributes);
> -    pango_attr_list_unref(attributes);
> -}
> -
> -static void make_label_bold(GtkLabel* label)
> -{
> -    PangoAttrList* attributes = pango_attr_list_new();
> -    pango_attr_list_insert(attributes, pango_attr_weight_new(PANGO_WEIGHT_BOLD));
> -    gtk_label_set_attributes(label, attributes);
> -    pango_attr_list_unref(attributes);
> -
> -}
> -
> -static gint
> -connect_dialog(GtkWindow *main_window, gchar **uri)
> -{
> -    GtkWidget *dialog, *area, *box, *label, *entry, *recent;
> -#if !GTK_CHECK_VERSION(3, 0, 0)
> -    GtkWidget *alignment;
> -#endif
> -    GtkRecentFilter *rfilter;
> -    gint retval;
> -
> -    /* Create the widgets */
> -    dialog = gtk_dialog_new_with_buttons(_("Connection details"),
> -                                         main_window,
> -                                         GTK_DIALOG_DESTROY_WITH_PARENT,
> -                                         GTK_STOCK_CANCEL,
> -                                         GTK_RESPONSE_REJECT,
> -                                         GTK_STOCK_CONNECT,
> -                                         GTK_RESPONSE_ACCEPT,
> -                                         NULL);
> -    gtk_dialog_set_default_response(GTK_DIALOG(dialog), GTK_RESPONSE_ACCEPT);
> -    gtk_container_set_border_width(GTK_CONTAINER(dialog), 5);
> -    area = gtk_dialog_get_content_area(GTK_DIALOG(dialog));
> -    box = gtk_vbox_new(FALSE, 6);
> -    gtk_container_set_border_width(GTK_CONTAINER(box), 5);
> -    gtk_box_pack_start(GTK_BOX(area), box, TRUE, TRUE, 0);
> -
> -    label = gtk_label_new_with_mnemonic(_("_Connection Address"));
> -    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> -    gtk_box_pack_start(GTK_BOX(box), label, TRUE, TRUE, 0);
> -    entry = GTK_WIDGET(gtk_entry_new());
> -    gtk_entry_set_activates_default(GTK_ENTRY(entry), TRUE);
> -    g_object_set(entry, "width-request", 200, NULL);
> -    g_signal_connect(entry, "changed", G_CALLBACK(entry_changed_cb), entry);
> -    g_signal_connect(entry, "icon-release", G_CALLBACK(entry_icon_release_cb), entry);
> -    gtk_box_pack_start(GTK_BOX(box), entry, TRUE, TRUE, 0);
> -    gtk_label_set_mnemonic_widget(GTK_LABEL(label), entry);
> -    make_label_bold(GTK_LABEL(label));
> -
> -    label = gtk_label_new(_("For example, spice://foo.example.org:5900"));
> -    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> -    make_label_light(GTK_LABEL(label));
> -#if GTK_CHECK_VERSION(3, 0, 0)
> -    gtk_box_pack_start(GTK_BOX(box), label, TRUE, TRUE, 0);
> -    gtk_widget_set_margin_bottom(label, 12);
> -#else
> -    alignment = gtk_alignment_new(0, 0, 1, 1);
> -    gtk_alignment_set_padding(GTK_ALIGNMENT(alignment), 0, 12, 0, 0);
> -    gtk_container_add(GTK_CONTAINER(alignment), label);
> -    gtk_box_pack_start(GTK_BOX(box), alignment, TRUE, TRUE, 0);
> -#endif
> -
> -    label = gtk_label_new_with_mnemonic(_("_Recent Connections"));
> -    make_label_bold(GTK_LABEL(label));
> -    gtk_box_pack_start(GTK_BOX(box), label, TRUE, TRUE, 0);
> -    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> -
> -    recent = GTK_WIDGET(gtk_recent_chooser_widget_new());
> -    gtk_recent_chooser_set_show_icons(GTK_RECENT_CHOOSER(recent), FALSE);
> -    gtk_recent_chooser_set_sort_type(GTK_RECENT_CHOOSER(recent), GTK_RECENT_SORT_MRU);
> -    gtk_box_pack_start(GTK_BOX(box), recent, TRUE, TRUE, 0);
> -    gtk_label_set_mnemonic_widget(GTK_LABEL(label), recent);
> -
> -    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(recent, "selection-changed",
> -                     G_CALLBACK(recent_selection_changed_dialog_cb), entry);
> -    g_signal_connect(recent, "item-activated",
> -                     G_CALLBACK(recent_item_activated_dialog_cb), dialog);
> -
> -    /* show and wait for response */
> -    gtk_widget_show_all(dialog);
> -    if (gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
> -        *uri = g_strdup(gtk_entry_get_text(GTK_ENTRY(entry)));
> -        g_strstrip(*uri);
> -        retval = 0;
> -    } else {
> -        *uri = NULL;
> -        retval = -1;
> -    }
> -    gtk_widget_destroy(dialog);
> -
> -    return retval;
> -}
> -
> -
>  #ifdef HAVE_OVIRT
>  static OvirtVm *
>  choose_vm(GtkWindow *main_window,
> @@ -1254,7 +1098,7 @@ remote_viewer_start(VirtViewerApp *app, GError **err)
>  retry_dialog:
>          main_window = virt_viewer_app_get_main_window(app);
>          if (priv->open_recent_dialog) {
> -            if (connect_dialog(virt_viewer_window_get_window(main_window), &guri) != 0) {
> +            if (remote_viewer_connect_dialog(virt_viewer_window_get_window(main_window), &guri) != 0) {
>                  g_set_error_literal(&error,
>                              VIRT_VIEWER_ERROR, VIRT_VIEWER_ERROR_CANCELLED,
>                              _("No connection was chosen"));
> --
> 2.4.2
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list





More information about the virt-tools-list mailing list