Re: [Qemu-devel] [PATCH] gtk: implement set_echo

[Gerd Hoffmann]

On So, 2016-02-21 at 21:36 +0100, Jan Kiszka wrote:
> On 2016-02-17 15:05, Paolo Bonzini wrote:
> > 
> > 
> > On 17/02/2016 14:53, Kevin Wolf wrote:
> >> Waiting didn't fix the bug, so I tried a git bisect now and it pointed
> >> me to this commit.
> >>
> >> I'm using HMP with the default vc backend. Starting with this commit,
> >> the echo is broken sometimes, in a way that the first character in the
> >> entered command is duplicated for each new character I enter, like this:
> >>
> >> (qemu) ii
> >> (qemu) iiin
> >> (qemu) nf
> >> (qemu) info
> >>
> >> This doesn't happen always, but often enough to be annoying.
> > 
> > The patch for it is already on the way.
> 
> Is it still on the road, or did it already arrive on the list at least?
> I just bisected to this bug as well and would be happy to see this fixed.

Pull req just sent.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] gtk: implement set_echo

[Jan Kiszka]

On 2016-02-17 15:05, Paolo Bonzini wrote:
> 
> 
> On 17/02/2016 14:53, Kevin Wolf wrote:
>> Waiting didn't fix the bug, so I tried a git bisect now and it pointed
>> me to this commit.
>>
>> I'm using HMP with the default vc backend. Starting with this commit,
>> the echo is broken sometimes, in a way that the first character in the
>> entered command is duplicated for each new character I enter, like this:
>>
>> (qemu) ii
>> (qemu) iiin
>> (qemu) nf
>> (qemu) info
>>
>> This doesn't happen always, but often enough to be annoying.
> 
> The patch for it is already on the way.

Is it still on the road, or did it already arrive on the list at least?
I just bisected to this bug as well and would be happy to see this fixed.

Thanks,
Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] gtk: implement set_echo

[Paolo Bonzini]

On 17/02/2016 14:53, Kevin Wolf wrote:
> Waiting didn't fix the bug, so I tried a git bisect now and it pointed
> me to this commit.
> 
> I'm using HMP with the default vc backend. Starting with this commit,
> the echo is broken sometimes, in a way that the first character in the
> entered command is duplicated for each new character I enter, like this:
> 
> (qemu) ii
> (qemu) iiin
> (qemu) nf
> (qemu) info
> 
> This doesn't happen always, but often enough to be annoying.

The patch for it is already on the way.

Paolo

Re: [Qemu-devel] [PATCH] gtk: implement set_echo

[Kevin Wolf]

Am 17.12.2015 um 13:47 hat Paolo Bonzini geschrieben:
> Even without line editing, this makes -qmp vc more pleasant with the
> GTK+ backend.  The only issue is that set_echo is invoked very early,
> long before a vc is actually associated with a VirtualConsole.  To work
> around this, create a temporary VirtualConsole until then.
> 
> Signed-off-by: Paolo Bonzini 

Waiting didn't fix the bug, so I tried a git bisect now and it pointed
me to this commit.

I'm using HMP with the default vc backend. Starting with this commit,
the echo is broken sometimes, in a way that the first character in the
entered command is duplicated for each new character I enter, like this:

(qemu) ii
(qemu) iiin
(qemu) nf
(qemu) info

This doesn't happen always, but often enough to be annoying.

Kevin

Re: [Qemu-devel] [PATCH] gtk: implement set_echo

[Gerd Hoffmann]

On Fr, 2016-01-08 at 14:11 +0100, Paolo Bonzini wrote:
> Ping.
> 
> Paolo
> 
> On 17/12/2015 13:47, Paolo Bonzini wrote:
> > Even without line editing, this makes -qmp vc more pleasant with the
> > GTK+ backend.  The only issue is that set_echo is invoked very early,
> > long before a vc is actually associated with a VirtualConsole.  To work
> > around this, create a temporary VirtualConsole until then.

Oops, slipped though.  Picked up now.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH] gtk: implement set_echo

[Paolo Bonzini]

Ping.

Paolo

On 17/12/2015 13:47, Paolo Bonzini wrote:
> Even without line editing, this makes -qmp vc more pleasant with the
> GTK+ backend.  The only issue is that set_echo is invoked very early,
> long before a vc is actually associated with a VirtualConsole.  To work
> around this, create a temporary VirtualConsole until then.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  include/ui/gtk.h |  1 +
>  ui/gtk.c | 45 -
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/include/ui/gtk.h b/include/ui/gtk.h
> index bf289cf..2bf60f3 100644
> --- a/include/ui/gtk.h
> +++ b/include/ui/gtk.h
> @@ -61,6 +61,7 @@ typedef struct VirtualVteConsole {
>  GtkWidget *scrollbar;
>  GtkWidget *terminal;
>  CharDriverState *chr;
> +bool echo;
>  } VirtualVteConsole;
>  #endif
>  
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 47b37e1..d9f85be 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1588,6 +1588,13 @@ static int gd_vc_chr_write(CharDriverState *chr, const 
> uint8_t *buf, int len)
>  return len;
>  }
>  
> +static void gd_vc_chr_set_echo(CharDriverState *chr, bool echo)
> +{
> +VirtualConsole *vc = chr->opaque;
> +
> +vc->vte.echo = echo;
> +}
> +
>  static int nb_vcs;
>  static CharDriverState *vcs[MAX_VCS];
>  
> @@ -1597,6 +1604,11 @@ static CharDriverState *gd_vc_handler(ChardevVC 
> *unused, Error **errp)
>  
>  chr = g_malloc0(sizeof(*chr));
>  chr->chr_write = gd_vc_chr_write;
> +chr->chr_set_echo = gd_vc_chr_set_echo;
> +
> +/* Temporary, until gd_vc_vte_init runs.  */
> +chr->opaque = g_new(VirtualConsole, 1);
> +
>  /* defer OPENED events until our vc is fully initialized */
>  chr->explicit_be_open = true;
>  
> @@ -1610,6 +1622,24 @@ static gboolean gd_vc_in(VteTerminal *terminal, gchar 
> *text, guint size,
>  {
>  VirtualConsole *vc = user_data;
>  
> +if (vc->vte.echo) {
> +VteTerminal *term = VTE_TERMINAL(vc->vte.terminal);
> +int i;
> +for (i = 0; i < size; i++) {
> +uint8_t c = text[i];
> +if (c >= 128 || isprint(c)) {
> +/* 8-bit characters are considered printable.  */
> +vte_terminal_feed(term, [i], 1);
> +} else if (c == '\r' || c == '\n') {
> +vte_terminal_feed(term, "\r\n", 2);
> +} else {
> +char ctrl[2] = { '^', 0};
> +ctrl[1] = text[i] ^ 64;
> +vte_terminal_feed(term, ctrl, 2);
> +}
> +}
> +}
> +
>  qemu_chr_be_write(vc->vte.chr, (uint8_t  *)text, (unsigned int)size);
>  return TRUE;
>  }
> @@ -1622,9 +1652,14 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, 
> VirtualConsole *vc,
>  GtkWidget *box;
>  GtkWidget *scrollbar;
>  GtkAdjustment *vadjustment;
> +VirtualConsole *tmp_vc = chr->opaque;
>  
>  vc->s = s;
> +vc->vte.echo = tmp_vc->vte.echo;
> +
>  vc->vte.chr = chr;
> +chr->opaque = vc;
> +g_free(tmp_vc);
>  
>  snprintf(buffer, sizeof(buffer), "vc%d", idx);
>  vc->label = g_strdup_printf("%s", vc->vte.chr->label
> @@ -1634,6 +1669,15 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, 
> VirtualConsole *vc,
>  vc->vte.terminal = vte_terminal_new();
>  g_signal_connect(vc->vte.terminal, "commit", G_CALLBACK(gd_vc_in), vc);
>  
> +/* The documentation says that the default is UTF-8, but actually it is
> + * 7-bit ASCII at least in VTE 0.38.
> + */
> +#if VTE_CHECK_VERSION(0, 40, 0)
> +vte_terminal_set_encoding(VTE_TERMINAL(vc->vte.terminal), "UTF-8", NULL);
> +#else
> +vte_terminal_set_encoding(VTE_TERMINAL(vc->vte.terminal), "UTF-8");
> +#endif
> +
>  vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->vte.terminal), -1);
>  vte_terminal_set_size(VTE_TERMINAL(vc->vte.terminal),
>VC_TERM_X_MIN, VC_TERM_Y_MIN);
> @@ -1656,7 +1700,6 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, 
> VirtualConsole *vc,
>  gtk_box_pack_start(GTK_BOX(box), vc->vte.terminal, TRUE, TRUE, 0);
>  gtk_box_pack_start(GTK_BOX(box), scrollbar, FALSE, FALSE, 0);
>  
> -vc->vte.chr->opaque = vc;
>  vc->vte.box = box;
>  vc->vte.scrollbar = scrollbar;
>  
> 

[Qemu-devel] [PATCH] gtk: implement set_echo

[Paolo Bonzini]

Even without line editing, this makes -qmp vc more pleasant with the
GTK+ backend.  The only issue is that set_echo is invoked very early,
long before a vc is actually associated with a VirtualConsole.  To work
around this, create a temporary VirtualConsole until then.

Signed-off-by: Paolo Bonzini 
---
 include/ui/gtk.h |  1 +
 ui/gtk.c | 45 -
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index bf289cf..2bf60f3 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -61,6 +61,7 @@ typedef struct VirtualVteConsole {
 GtkWidget *scrollbar;
 GtkWidget *terminal;
 CharDriverState *chr;
+bool echo;
 } VirtualVteConsole;
 #endif
 
diff --git a/ui/gtk.c b/ui/gtk.c
index 47b37e1..d9f85be 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1588,6 +1588,13 @@ static int gd_vc_chr_write(CharDriverState *chr, const 
uint8_t *buf, int len)
 return len;
 }
 
+static void gd_vc_chr_set_echo(CharDriverState *chr, bool echo)
+{
+VirtualConsole *vc = chr->opaque;
+
+vc->vte.echo = echo;
+}
+
 static int nb_vcs;
 static CharDriverState *vcs[MAX_VCS];
 
@@ -1597,6 +1604,11 @@ static CharDriverState *gd_vc_handler(ChardevVC *unused, 
Error **errp)
 
 chr = g_malloc0(sizeof(*chr));
 chr->chr_write = gd_vc_chr_write;
+chr->chr_set_echo = gd_vc_chr_set_echo;
+
+/* Temporary, until gd_vc_vte_init runs.  */
+chr->opaque = g_new(VirtualConsole, 1);
+
 /* defer OPENED events until our vc is fully initialized */
 chr->explicit_be_open = true;
 
@@ -1610,6 +1622,24 @@ static gboolean gd_vc_in(VteTerminal *terminal, gchar 
*text, guint size,
 {
 VirtualConsole *vc = user_data;
 
+if (vc->vte.echo) {
+VteTerminal *term = VTE_TERMINAL(vc->vte.terminal);
+int i;
+for (i = 0; i < size; i++) {
+uint8_t c = text[i];
+if (c >= 128 || isprint(c)) {
+/* 8-bit characters are considered printable.  */
+vte_terminal_feed(term, [i], 1);
+} else if (c == '\r' || c == '\n') {
+vte_terminal_feed(term, "\r\n", 2);
+} else {
+char ctrl[2] = { '^', 0};
+ctrl[1] = text[i] ^ 64;
+vte_terminal_feed(term, ctrl, 2);
+}
+}
+}
+
 qemu_chr_be_write(vc->vte.chr, (uint8_t  *)text, (unsigned int)size);
 return TRUE;
 }
@@ -1622,9 +1652,14 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, 
VirtualConsole *vc,
 GtkWidget *box;
 GtkWidget *scrollbar;
 GtkAdjustment *vadjustment;
+VirtualConsole *tmp_vc = chr->opaque;
 
 vc->s = s;
+vc->vte.echo = tmp_vc->vte.echo;
+
 vc->vte.chr = chr;
+chr->opaque = vc;
+g_free(tmp_vc);
 
 snprintf(buffer, sizeof(buffer), "vc%d", idx);
 vc->label = g_strdup_printf("%s", vc->vte.chr->label
@@ -1634,6 +1669,15 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, 
VirtualConsole *vc,
 vc->vte.terminal = vte_terminal_new();
 g_signal_connect(vc->vte.terminal, "commit", G_CALLBACK(gd_vc_in), vc);
 
+/* The documentation says that the default is UTF-8, but actually it is
+ * 7-bit ASCII at least in VTE 0.38.
+ */
+#if VTE_CHECK_VERSION(0, 40, 0)
+vte_terminal_set_encoding(VTE_TERMINAL(vc->vte.terminal), "UTF-8", NULL);
+#else
+vte_terminal_set_encoding(VTE_TERMINAL(vc->vte.terminal), "UTF-8");
+#endif
+
 vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->vte.terminal), -1);
 vte_terminal_set_size(VTE_TERMINAL(vc->vte.terminal),
   VC_TERM_X_MIN, VC_TERM_Y_MIN);
@@ -1656,7 +1700,6 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, 
VirtualConsole *vc,
 gtk_box_pack_start(GTK_BOX(box), vc->vte.terminal, TRUE, TRUE, 0);
 gtk_box_pack_start(GTK_BOX(box), scrollbar, FALSE, FALSE, 0);
 
-vc->vte.chr->opaque = vc;
 vc->vte.box = box;
 vc->vte.scrollbar = scrollbar;
 
-- 
2.5.0