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

2017-08-10 Thread Marc-André Lureau
Hi

- Original Message -
> Hi,
> 
> On Wed, Aug 09, 2017 at 10:49:51PM +0200, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > 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 
> 
> 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..000
> > --- 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
> > .
> > -*/
> > -
> > -#include 
> > -#include 
> > -#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));

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

2017-08-09 Thread Victor Toso
Hi,

On Wed, Aug 09, 2017 at 10:49:51PM +0200, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
>
> 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 

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..000
> --- 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 
> .
> -*/
> -
> -#include 
> -#include 
> -#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

[Spice-devel] [PATCH spice-gtk] spicy: remove connect dialog

2017-08-09 Thread marcandre . lureau
From: Marc-André Lureau 

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.

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.

Signed-off-by: Marc-André Lureau 
---
 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..000
--- 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 .
-*/
-
-#include 
-#include 
-#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_c