Re: [PATCH spice-gtk] spicy: remove connect dialog

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Aug 09, 2017 at 10:49:51PM +0200, marcandre.lureau@xxxxxxxxxx wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> The connect dialog isn't working properly when recent entries are for
> spice+unix://. We could improve the recent dialog, but spicy isn't
> meant to be a fully-feature spice client.

Well, it is useful for recent entries ...

> It makes user believe that it is a end-user application. It is not, it
> is only meant to do testing. So if you need to lookup recent
> connections, you can use the shell history instead.

The user should prefer other applications for what they provide,
removing this here to make them use something else is a bit weird.

> Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>

But If no one else complains, I'm fine with it... and patch looks fine.

> ---
>  doc/reference/Makefile.am |   1 -
>  tools/Makefile.am         |   2 -
>  tools/spicy-connect.c     | 248 ----------------------------------------------
>  tools/spicy-connect.h     |  26 -----
>  tools/spicy.c             | 115 +++------------------
>  5 files changed, 15 insertions(+), 377 deletions(-)
>  delete mode 100644 tools/spicy-connect.c
>  delete mode 100644 tools/spicy-connect.h
> 
> diff --git a/doc/reference/Makefile.am b/doc/reference/Makefile.am
> index 9fda456..cc7abe7 100644
> --- a/doc/reference/Makefile.am
> +++ b/doc/reference/Makefile.am
> @@ -50,7 +50,6 @@ IGNORE_HFILES=					\
>  	spice-uri-priv.h			\
>  	spice-util-priv.h			\
>  	spice-widget-priv.h			\
> -	spicy-connect.h				\
>  	usb-acl-helper.h			\
>  	usb-device-manager-priv.h		\
>  	usbdk_api.h				\
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 52523e1..5b7c4de 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -19,8 +19,6 @@ endif
>  
>  spicy_SOURCES =				\
>  	spicy.c				\
> -	spicy-connect.h 		\
> -	spicy-connect.c 		\
>  	spice-cmdline.h			\
>  	spice-cmdline.c			\
>  	$(NULL)
> diff --git a/tools/spicy-connect.c b/tools/spicy-connect.c
> deleted file mode 100644
> index 39555a6..0000000
> --- a/tools/spicy-connect.c
> +++ /dev/null
> @@ -1,248 +0,0 @@
> -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> -/*
> -   Copyright (C) 2010-2015 Red Hat, Inc.
> -
> -   This library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   This library 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
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with this library; if not, see <http://www.gnu.org/licenses/>.
> -*/
> -
> -#include <gtk/gtk.h>
> -#include <gdk/gdkkeysyms.h>
> -#include "spice-common.h"
> -#include "spicy-connect.h"
> -
> -typedef struct
> -{
> -    gboolean connecting;
> -    GMainLoop *loop;
> -    SpiceSession *session;
> -} ConnectionInfo;
> -
> -static struct {
> -    const char *text;
> -    const char *prop;
> -    GtkWidget *entry;
> -} connect_entries[] = {
> -    { .text = "Hostname",   .prop = "host"      },
> -    { .text = "Port",       .prop = "port"      },
> -    { .text = "TLS Port",   .prop = "tls-port"  },
> -};
> -
> -static gboolean can_connect(void)
> -{
> -    if ((gtk_entry_get_text_length(GTK_ENTRY(connect_entries[0].entry)) > 0) &&
> -        ((gtk_entry_get_text_length(GTK_ENTRY(connect_entries[1].entry)) > 0) ||
> -         (gtk_entry_get_text_length(GTK_ENTRY(connect_entries[2].entry)) > 0)))
> -        return TRUE;
> -
> -    return FALSE;
> -}
> -
> -static void set_connection_info(SpiceSession *session)
> -{
> -    const gchar *txt;
> -    int i;
> -
> -    for (i = 0; i < SPICE_N_ELEMENTS(connect_entries); i++) {
> -        txt = gtk_entry_get_text(GTK_ENTRY(connect_entries[i].entry));
> -        g_object_set(session, connect_entries[i].prop, txt, NULL);
> -    }
> -}
> -
> -static gboolean close_cb(gpointer data)
> -{
> -    ConnectionInfo *info = data;
> -    info->connecting = FALSE;
> -    if (g_main_loop_is_running(info->loop))
> -        g_main_loop_quit(info->loop);
> -
> -    return TRUE;
> -}
> -
> -static void entry_changed_cb(GtkEditable* entry, gpointer data)
> -{
> -    GtkButton *connect_button = data;
> -    gtk_widget_set_sensitive(GTK_WIDGET(connect_button), can_connect());
> -}
> -
> -static gboolean entry_focus_in_cb(GtkWidget *widget, GdkEvent *event, gpointer data)
> -{
> -    GtkRecentChooser *recent = GTK_RECENT_CHOOSER(data);
> -    gtk_recent_chooser_unselect_all(recent);
> -    return TRUE;
> -}
> -
> -static gboolean key_pressed_cb(GtkWidget *widget, GdkEvent *event, gpointer data)
> -{
> -    gboolean tst;
> -    if (event->type == GDK_KEY_PRESS) {
> -        switch (event->key.keyval) {
> -            case GDK_KEY_Escape:
> -                g_signal_emit_by_name(GTK_WIDGET(data), "delete-event", NULL, &tst);
> -                return TRUE;
> -            default:
> -                return FALSE;
> -        }
> -    }
> -
> -    return FALSE;
> -}
> -
> -static void recent_selection_changed_dialog_cb(GtkRecentChooser *chooser, gpointer data)
> -{
> -    GtkRecentInfo *info;
> -    gchar *txt = NULL;
> -    const gchar *uri;
> -    SpiceSession *session = data;
> -    int i;
> -
> -    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);
> -
> -    g_object_set(session, "uri", uri, NULL);
> -
> -    for (i = 0; i < SPICE_N_ELEMENTS(connect_entries); i++) {
> -        g_object_get(session, connect_entries[i].prop, &txt, NULL);
> -        gtk_entry_set_text(GTK_ENTRY(connect_entries[i].entry), txt ? txt : "");
> -        g_free(txt);
> -    }
> -
> -    gtk_recent_info_unref(info);
> -}
> -
> -static void connect_cb(gpointer data)
> -{
> -    ConnectionInfo *info = data;
> -    if (can_connect())
> -    {
> -        info->connecting = TRUE;
> -        set_connection_info(info->session);
> -        if (g_main_loop_is_running(info->loop))
> -            g_main_loop_quit(info->loop);
> -    }
> -}
> -
> -gboolean spicy_connect_dialog(SpiceSession *session)
> -{
> -    GtkWidget *connect_button, *cancel_button, *label;
> -    GtkBox *main_box, *recent_box, *button_box;
> -    GtkWindow *window;
> -    GtkGrid *grid;
> -    int i;
> -
> -    ConnectionInfo info = {
> -        FALSE,
> -        NULL,
> -        session
> -    };
> -
> -    /* Create the widgets */
> -    window = GTK_WINDOW(gtk_window_new(GTK_WINDOW_TOPLEVEL));
> -    gtk_window_set_title(window, "Connect to SPICE");
> -    gtk_window_set_resizable(window, FALSE);
> -    gtk_container_set_border_width(GTK_CONTAINER(window), 5);
> -
> -    main_box = GTK_BOX(gtk_box_new(GTK_ORIENTATION_VERTICAL, 0));
> -    gtk_container_add(GTK_CONTAINER(window), GTK_WIDGET(main_box));
> -
> -    grid = GTK_GRID(gtk_grid_new());
> -    gtk_box_pack_start(main_box, GTK_WIDGET(grid), FALSE, TRUE, 0);
> -    gtk_container_set_border_width(GTK_CONTAINER(grid), 5);
> -    gtk_grid_set_row_spacing(grid, 5);
> -    gtk_grid_set_column_spacing(grid, 5);
> -
> -    for (i = 0; i < SPICE_N_ELEMENTS(connect_entries); i++) {
> -        gchar *txt;
> -        label = gtk_label_new(connect_entries[i].text);
> -        gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> -        gtk_grid_attach(grid, label, 0, i, 1, 1);
> -        connect_entries[i].entry = GTK_WIDGET(gtk_entry_new());
> -        gtk_grid_attach(grid, connect_entries[i].entry, 1, i, 1, 1);
> -        g_object_get(session, connect_entries[i].prop, &txt, NULL);
> -        SPICE_DEBUG("%s: #%i [%s]: \"%s\"",
> -                __FUNCTION__, i, connect_entries[i].prop, txt);
> -        if (txt) {
> -            gtk_entry_set_text(GTK_ENTRY(connect_entries[i].entry), txt);
> -            g_free(txt);
> -        }
> -    }
> -
> -    recent_box = GTK_BOX(gtk_box_new(GTK_ORIENTATION_VERTICAL, 0));
> -    gtk_box_pack_start(main_box, GTK_WIDGET(recent_box), TRUE, TRUE, 0);
> -    gtk_container_set_border_width(GTK_CONTAINER(recent_box), 5);
> -
> -    label = gtk_label_new("Recent connections:");
> -    gtk_box_pack_start(recent_box, label, FALSE, TRUE, 0);
> -    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
> -
> -    button_box = GTK_BOX(gtk_button_box_new(GTK_ORIENTATION_HORIZONTAL));
> -    gtk_button_box_set_layout(GTK_BUTTON_BOX(button_box), GTK_BUTTONBOX_END);
> -    gtk_box_set_spacing(button_box, 5);
> -    gtk_container_set_border_width(GTK_CONTAINER(button_box), 5);
> -    connect_button = gtk_button_new_with_label("Connect");
> -    cancel_button = gtk_button_new_with_label("Cancel");
> -    gtk_box_pack_start(button_box, cancel_button, FALSE, TRUE, 0);
> -    gtk_box_pack_start(button_box, connect_button, FALSE, TRUE, 1);
> -
> -    gtk_box_pack_start(main_box, GTK_WIDGET(button_box), FALSE, TRUE, 0);
> -
> -    gtk_widget_set_sensitive(GTK_WIDGET(connect_button), can_connect());
> -
> -    g_signal_connect(window, "key-press-event",
> -                     G_CALLBACK(key_pressed_cb), window);
> -    g_signal_connect_swapped(window, "delete-event",
> -                             G_CALLBACK(close_cb), &info);
> -    g_signal_connect_swapped(connect_button, "clicked",
> -                             G_CALLBACK(connect_cb), &info);
> -    g_signal_connect_swapped(cancel_button, "clicked",
> -                             G_CALLBACK(close_cb), &info);
> -
> -    GtkRecentFilter *rfilter;
> -    GtkWidget *recent;
> -
> -    recent = GTK_WIDGET(gtk_recent_chooser_widget_new());
> -    gtk_recent_chooser_set_show_icons(GTK_RECENT_CHOOSER(recent), FALSE);
> -    gtk_box_pack_start(recent_box, recent, TRUE, TRUE, 0);
> -
> -    rfilter = gtk_recent_filter_new();
> -    gtk_recent_filter_add_mime_type(rfilter, "application/x-spice");
> -    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), session);
> -    g_signal_connect_swapped(recent, "item-activated",
> -                             G_CALLBACK(connect_cb), &info);
> -
> -    for (i = 0; i < SPICE_N_ELEMENTS(connect_entries); i++) {
> -        g_signal_connect_swapped(connect_entries[i].entry, "activate",
> -                                 G_CALLBACK(connect_cb), &info);
> -        g_signal_connect(connect_entries[i].entry, "changed",
> -                         G_CALLBACK(entry_changed_cb), connect_button);
> -        g_signal_connect(connect_entries[i].entry, "focus-in-event",
> -                         G_CALLBACK(entry_focus_in_cb), recent);
> -    }
> -
> -    /* show and wait for response */
> -    gtk_widget_show_all(GTK_WIDGET(window));
> -
> -    info.loop = g_main_loop_new(NULL, FALSE);
> -    g_main_loop_run(info.loop);
> -
> -    gtk_widget_destroy(GTK_WIDGET(window));
> -
> -    return info.connecting;
> -}
> diff --git a/tools/spicy-connect.h b/tools/spicy-connect.h
> deleted file mode 100644
> index 56b2d80..0000000
> --- a/tools/spicy-connect.h
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> -/*
> -   Copyright (C) 2010-2015 Red Hat, Inc.
> -
> -   This library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   This library 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
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with this library; if not, see <http://www.gnu.org/licenses/>.
> -*/
> -
> -#ifndef SPICY_CONNECT_H
> -#define SPICY_CONNECT_H
> -
> -#include "spice-widget.h"
> -
> -gboolean spicy_connect_dialog(SpiceSession *session);
> -
> -#endif
> diff --git a/tools/spicy.c b/tools/spicy.c
> index 40cd6b3..5e6a620 100644
> --- a/tools/spicy.c
> +++ b/tools/spicy.c
> @@ -36,7 +36,6 @@
>  #include "spice-option.h"
>  #include "usb-device-widget.h"
>  
> -#include "spicy-connect.h"
>  #if HAVE_GSTAUDIO || HAVE_GSTVIDEO
>  #include <gst/gst.h>
>  #endif
> @@ -67,7 +66,6 @@ struct _SpiceWindow {
>      gint             monitor_id;
>      GtkWidget        *toplevel, *spice;
>      GtkWidget        *menubar, *toolbar;
> -    GtkWidget        *ritem, *rmenu;
>      GtkWidget        *statusbar, *status, *st[STATE_MAX];
>      GtkActionGroup   *ag;
>      GtkUIManager     *ui;
> @@ -252,14 +250,6 @@ static void update_edit_menu(struct spice_connection *conn)
>      }
>  }
>  
> -static void menu_cb_connect(GtkAction *action, void *data)
> -{
> -    struct spice_connection *conn;
> -
> -    conn = connection_new();
> -    connection_connect(conn);
> -}
> -
>  static void menu_cb_close(GtkAction *action, void *data)
>  {
>      SpiceWindow *win = data;
> @@ -692,9 +682,6 @@ static const GtkActionEntry entries[] = {
>      {
>          .name        = "FileMenu",
>          .label       = "_File",
> -    },{
> -        .name        = "FileRecentMenu",
> -        .label       = "_Recent",
>      },{
>          .name        = "EditMenu",
>          .label       = "_Edit",
> @@ -717,13 +704,6 @@ static const GtkActionEntry entries[] = {
>          .name        = "HelpMenu",
>          .label       = "_Help",
>      },{
> -
> -        /* File menu */
> -        .name        = "Connect",
> -        .stock_id    = "_Connect",
> -        .label       = "_Connect ...",
> -        .callback    = G_CALLBACK(menu_cb_connect),
> -    },{
>          .name        = "Close",
>          .stock_id    = "window-close",
>          .label       = "_Close",
> @@ -908,9 +888,6 @@ static char ui_xml[] =
>  "<ui>\n"
>  "  <menubar action='MainMenu'>\n"
>  "    <menu action='FileMenu'>\n"
> -"      <menuitem action='Connect'/>\n"
> -"      <menu action='FileRecentMenu'/>\n"
> -"      <separator/>\n"
>  "      <menuitem action='Close'/>\n"
>  "    </menu>\n"
>  "    <menu action='EditMenu'>\n"
> @@ -988,23 +965,6 @@ static gboolean is_gtk_session_property(const gchar *property)
>      return FALSE;
>  }
>  
> -static void recent_item_activated_cb(GtkRecentChooser *chooser, gpointer data)
> -{
> -    GtkRecentInfo *info;
> -    struct spice_connection *conn;
> -    const char *uri;
> -
> -    info = gtk_recent_chooser_get_current_item(chooser);
> -
> -    uri = gtk_recent_info_get_uri(info);
> -    g_return_if_fail(uri != NULL);
> -
> -    conn = connection_new();
> -    g_object_set(conn->session, "uri", uri, NULL);
> -    gtk_recent_info_unref(info);
> -    connection_connect(conn);
> -}
> -
>  static void compression_cb(GtkRadioAction *action G_GNUC_UNUSED,
>                             GtkRadioAction *current,
>                             gpointer user_data)
> @@ -1098,22 +1058,6 @@ static SpiceWindow *create_spice_window(spice_connection *conn, SpiceChannel *ch
>      win->menubar = gtk_ui_manager_get_widget(win->ui, "/MainMenu");
>      win->toolbar = gtk_ui_manager_get_widget(win->ui, "/ToolBar");
>  
> -    /* recent menu */
> -    win->ritem  = gtk_ui_manager_get_widget
> -        (win->ui, "/MainMenu/FileMenu/FileRecentMenu");
> -
> -    GtkRecentFilter  *rfilter;
> -
> -    win->rmenu = gtk_recent_chooser_menu_new();
> -    gtk_recent_chooser_set_show_icons(GTK_RECENT_CHOOSER(win->rmenu), FALSE);
> -    rfilter = gtk_recent_filter_new();
> -    gtk_recent_filter_add_mime_type(rfilter, "application/x-spice");
> -    gtk_recent_chooser_add_filter(GTK_RECENT_CHOOSER(win->rmenu), rfilter);
> -    gtk_recent_chooser_set_local_only(GTK_RECENT_CHOOSER(win->rmenu), FALSE);
> -    gtk_menu_item_set_submenu(GTK_MENU_ITEM(win->ritem), win->rmenu);
> -    g_signal_connect(win->rmenu, "item-activated",
> -                     G_CALLBACK(recent_item_activated_cb), win);
> -
>      /* spice display */
>      win->spice = GTK_WIDGET(spice_display_new_with_monitor(conn->session, id, monitor_id));
>      seq = spice_grab_sequence_new_from_string("Shift_L+F12");
> @@ -1238,35 +1182,6 @@ static void destroy_spice_window(SpiceWindow *win)
>      g_object_unref(win);
>  }
>  
> -/* ------------------------------------------------------------------ */
> -
> -static void recent_add(SpiceSession *session)
> -{
> -    GtkRecentManager *recent;
> -    GtkRecentData meta = {
> -        .mime_type    = (char*)"application/x-spice",
> -        .app_name     = (char*)"spicy",
> -        .app_exec     = (char*)"spicy --uri=%u",
> -    };
> -    char *uri;
> -
> -    g_object_get(session, "uri", &uri, NULL);
> -    SPICE_DEBUG("%s: %s", __FUNCTION__, uri);
> -
> -    recent = gtk_recent_manager_get_default();
> -    if (g_str_has_prefix(uri, "spice://"))
> -        meta.display_name = uri + 8;
> -    else if (g_str_has_prefix(uri, "spice+unix://"))
> -        meta.display_name = uri + 13;
> -    else
> -        g_return_if_reached();
> -
> -    if (!gtk_recent_manager_add_full(recent, uri, &meta))
> -        g_warning("Recent item couldn't be added successfully");
> -
> -    g_free(uri);
> -}
> -
>  static void main_channel_event(SpiceChannel *channel, SpiceChannelEvent event,
>                                 gpointer data)
>  {
> @@ -1278,7 +1193,6 @@ static void main_channel_event(SpiceChannel *channel, SpiceChannelEvent event,
>      switch (event) {
>      case SPICE_CHANNEL_OPENED:
>          g_message("main channel: opened");
> -        recent_add(conn->session);
>          break;
>      case SPICE_CHANNEL_SWITCHING:
>          g_message("main channel: switching host");
> @@ -1300,11 +1214,7 @@ static void main_channel_event(SpiceChannel *channel, SpiceChannelEvent event,
>              g_message("channel error: %s", error->message);
>          }
>  
> -        if (spicy_connect_dialog(conn->session)) {
> -            connection_connect(conn);
> -        } else {
> -            connection_disconnect(conn);
> -        }
> +        connection_disconnect(conn);
>          break;
>      case SPICE_CHANNEL_ERROR_AUTH:
>          g_warning("main channel: auth failure (wrong password?)");
> @@ -1980,6 +1890,7 @@ int main(int argc, char *argv[])
>      spice_connection *conn;
>      gchar *conf_file, *conf;
>      char *host = NULL, *port = NULL, *tls_port = NULL, *unix_path = NULL;
> +    int ret = 1;
>  
>      keyfile = g_key_file_new();
>  
> @@ -2013,13 +1924,13 @@ int main(int argc, char *argv[])
>  #endif
>      if (!g_option_context_parse (context, &argc, &argv, &error)) {
>          g_print("option parsing failed: %s\n", error->message);
> -        exit(1);
> +        goto end;
>      }
> -    g_option_context_free(context);
>  
>      if (version) {
>          g_print("spicy " PACKAGE_VERSION "\n");
> -        exit(0);
> +        ret = 0;
> +        goto end;
>      }
>  
>      mainloop = g_main_loop_new(NULL, false);
> @@ -2034,12 +1945,12 @@ int main(int argc, char *argv[])
>                   "port", &port,
>                   "tls-port", &tls_port,
>                   NULL);
> -    /* If user doesn't provide hostname and port, show the dialog window
> -       instead of connecting to server automatically */
> +    /* If user doesn't provide hostname and port, exit */
>      if ((host == NULL || (port == NULL && tls_port == NULL)) && unix_path == NULL) {
> -        if (!spicy_connect_dialog(conn->session)) {
> -            exit(0);
> -        }
> +        char *help = g_option_context_get_help(context, TRUE, NULL);
> +        g_print("%s", help);
> +        g_free(help);
> +        goto end;
>      }
>      g_free(host);
>      g_free(port);
> @@ -2065,5 +1976,9 @@ int main(int argc, char *argv[])
>      g_free(spicy_title);
>  
>      setup_terminal(true);
> -    return 0;
> +    ret = 0;
> +
> +end:
> +    g_option_context_free(context);
> +    return ret;
>  }
> -- 
> 2.14.0.1.geff633fa0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]