Hi ----- Original Message ----- > 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. It's easy to complain if you are used to it. But there is too much duplication of effort in the various Spice clients and spicy is clearly one of the worst client. Since it's main reason to be is for simple testing, I would stop installing it on system, and remove the extra features: what isn't meant for testing shouldn't be there or added, we are not trying to compete with better clients. > > > --- > > 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 > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel