[PATCHv3 1/2] Initial TUI mouse support

[PATCHv3 1/2] Initial TUI mouse support

[Hannes Domani]

Implements an overridable tui_win_info::click method whose arguments
are the mouse coordinates inside the specific window, and the mouse
button clicked.

And if the curses implementation supports 5 buttons, the 4th and 5th
buttons are used for scrolling.

2021-06-03  Hannes Domani  <ssbssa@yahoo.de>

	* ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
	* tui/tui-data.h (struct tui_win_info): Add click function.
	* tui/tui-io.c (tui_prep_terminal): Enable mouse events.
	(tui_deprep_terminal): Disable mouse events.
	(tui_dispatch_ctrl_char): Handle KEY_MOUSE.
	* tui/tui.c (tui_disable): Disable mouse events.
---
v2:
- Added ChangeLog.
v3:
- Use NCURSES_MOUSE_VERSION to check if mouse functions are available.
- Mention possible mouse_button values in click function.
- Remove useless #include "tui/tui-source.h"
- Fix code style issues.
---
 gdb/ser-mingw.c    |  5 +++++
 gdb/tui/tui-data.h |  7 +++++++
 gdb/tui/tui-io.c   | 37 +++++++++++++++++++++++++++++++++++++
 gdb/tui/tui.c      |  4 ++++
 4 files changed, 53 insertions(+)

diff --git a/gdb/ser-mingw.c b/gdb/ser-mingw.c
index 043bb50b577..2bad51310f6 100644
--- a/gdb/ser-mingw.c
+++ b/gdb/ser-mingw.c
@@ -599,6 +599,11 @@ console_select_thread (void *arg)
 		  break;
 		}
 	    }
+	  else if (record.EventType == MOUSE_EVENT)
+	    {
+	      SetEvent (state->read_event);
+	      break;
+	    }
 
 	  /* Otherwise discard it and wait again.  */
 	  ReadConsoleInput (h, &record, 1, &n_records);
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index b4d788dd0a4..9fa39fa58f3 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -137,6 +137,13 @@ struct tui_win_info
     return true;
   }
 
+  /* Called for each mouse click inside this window.  Coordinates MOUSE_X
+     and MOUSE_Y are 0-based relative to the window, and MOUSE_BUTTON can
+     be 1 (left), 2 (middle), or 3 (right).  */
+  virtual void click (int mouse_x, int mouse_y, int mouse_button)
+  {
+  }
+
   void check_and_display_highlight_if_needed ();
 
   /* Window handle.  */
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index a2be4d4353e..7df0e2f1bd3 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -639,6 +639,9 @@ tui_redisplay_readline (void)
 static void
 tui_prep_terminal (int notused1)
 {
+#ifdef NCURSES_MOUSE_VERSION
+  mousemask (ALL_MOUSE_EVENTS, NULL);
+#endif
 }
 
 /* Readline callback to restore the terminal.  It is called once each
@@ -646,6 +649,9 @@ tui_prep_terminal (int notused1)
 static void
 tui_deprep_terminal (void)
 {
+#ifdef NCURSES_MOUSE_VERSION
+  mousemask (0, NULL);
+#endif
 }
 
 #ifdef TUI_USE_PIPE_FOR_READLINE
@@ -978,6 +984,37 @@ tui_dispatch_ctrl_char (unsigned int ch)
     case KEY_LEFT:
       win_info->right_scroll (1);
       break;
+#ifdef NCURSES_MOUSE_VERSION
+    case KEY_MOUSE:
+	{
+	  MEVENT mev;
+	  if (getmouse (&mev) != OK)
+	    break;
+
+	  for (tui_win_info *wi : all_tui_windows ())
+	    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
+		&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
+	      {
+		if ((mev.bstate & BUTTON1_CLICKED) != 0
+		    || (mev.bstate & BUTTON2_CLICKED) != 0
+		    || (mev.bstate & BUTTON3_CLICKED) != 0)
+		  {
+		    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
+		      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
+				 : 3);
+		    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
+		  }
+#ifdef BUTTON5_PRESSED
+		else if ((mev.bstate & BUTTON4_PRESSED) != 0)
+		  wi->backward_scroll (3);
+		else if ((mev.bstate & BUTTON5_PRESSED) != 0)
+		  wi->forward_scroll (3);
+#endif
+		break;
+	      }
+	}
+      break;
+#endif
     case '\f':
       break;
     default:
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index af92b2a8042..529fc62c9ac 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -508,6 +508,10 @@ tui_disable (void)
   rl_startup_hook = 0;
   rl_already_prompted = 0;
 
+#ifdef NCURSES_MOUSE_VERSION
+  mousemask (0, NULL);
+#endif
+
   /* Leave curses and restore previous gdb terminal setting.  */
   endwin ();
 
-- 
2.31.1

[PATCHv3 2/2] Forward mouse click to python TUI window

[Hannes Domani]

If the TUI window object implements the click method, it is called for each
mouse click event in this window.

gdb/ChangeLog:

2021-06-03  Hannes Domani  <ssbssa@yahoo.de>

	* python/py-tui.c (class tui_py_window): Add click function.
	(tui_py_window::click): Likewise.

gdb/doc/ChangeLog:

2021-06-03  Hannes Domani  <ssbssa@yahoo.de>

	* python.texi (TUI Windows In Python): Document Window.click.
---
v2:
- Added ChangeLog.
- Specify in the documentation that mouse coordinates are 0-based.
v3:
- Mention possible button values.
---
 gdb/doc/python.texi |  7 +++++++
 gdb/python/py-tui.c | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 0d8f480e472..7574d29f2c2 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -6022,6 +6022,13 @@ contents.  A positive argument should cause the viewport to move down,
 and so the content should appear to move up.
 @end defun
 
+@defun Window.click (@var{x}, @var{y}, @var{button})
+This is called on a mouse click in this window.  @var{x} and @var{y} are
+the mouse coordinates inside the window (0-based), and @var{button}
+specifies which mouse button was used, whose values can be 1 (left),
+2 (middle), or 3 (right).
+@end defun
+
 @node Python Auto-loading
 @subsection Python Auto-loading
 @cindex Python auto-loading
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 97e9de7a00c..8dfed9d341f 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -101,6 +101,8 @@ class tui_py_window : public tui_win_info
       tui_win_info::refresh_window ();
   }
 
+  void click (int mouse_x, int mouse_y, int mouse_button) override;
+
   /* Erase and re-box the window.  */
   void erase ()
   {
@@ -229,6 +231,21 @@ tui_py_window::do_scroll_vertical (int num_to_scroll)
     }
 }
 
+void
+tui_py_window::click (int mouse_x, int mouse_y, int mouse_button)
+{
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (PyObject_HasAttrString (m_window.get (), "click"))
+    {
+      gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "click",
+					       "iii", mouse_x, mouse_y,
+					       mouse_button));
+      if (result == nullptr)
+	gdbpy_print_stack ();
+    }
+}
+
 void
 tui_py_window::output (const char *text, bool full_window)
 {
-- 
2.31.1

Re: [PATCHv3 2/2] Forward mouse click to python TUI window

[Eli Zaretskii]

> Date: Thu,  3 Jun 2021 17:14:53 +0200
> From: Hannes Domani via Gdb-patches <gdb-patches@sourceware.org>
> 
> If the TUI window object implements the click method, it is called for each
> mouse click event in this window.
> 
> gdb/ChangeLog:
> 
> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* python/py-tui.c (class tui_py_window): Add click function.
> 	(tui_py_window::click): Likewise.
> 
> gdb/doc/ChangeLog:
> 
> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* python.texi (TUI Windows In Python): Document Window.click.

Thanks, the documentation part is OK.

Re: [PATCHv3 2/2] Forward mouse click to python TUI window

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> If the TUI window object implements the click method, it is called for each
Hannes> mouse click event in this window.

Hannes> gdb/ChangeLog:

Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>

Hannes> 	* python/py-tui.c (class tui_py_window): Add click function.
Hannes> 	(tui_py_window::click): Likewise.

Hannes> gdb/doc/ChangeLog:

Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>

Hannes> 	* python.texi (TUI Windows In Python): Document Window.click.

Thanks for the update.  Looks good.

Tom

Re: [PATCHv3 1/2] Initial TUI mouse support

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> Implements an overridable tui_win_info::click method whose arguments
Hannes> are the mouse coordinates inside the specific window, and the mouse
Hannes> button clicked.

Hannes> And if the curses implementation supports 5 buttons, the 4th and 5th
Hannes> buttons are used for scrolling.

Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>

Hannes> 	* ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
Hannes> 	* tui/tui-data.h (struct tui_win_info): Add click function.
Hannes> 	* tui/tui-io.c (tui_prep_terminal): Enable mouse events.
Hannes> 	(tui_deprep_terminal): Disable mouse events.
Hannes> 	(tui_dispatch_ctrl_char): Handle KEY_MOUSE.
Hannes> 	* tui/tui.c (tui_disable): Disable mouse events.

Looks good.  Thank you again.

Tom

Re: [PATCHv3 1/2] Initial TUI mouse support

[Hannes Domani]

 Am Freitag, 4. Juni 2021, 15:51:27 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Hannes> Implements an overridable tui_win_info::click method whose arguments
> Hannes> are the mouse coordinates inside the specific window, and the mouse
> Hannes> button clicked.
>
> Hannes> And if the curses implementation supports 5 buttons, the 4th and 5th
> Hannes> buttons are used for scrolling.
>
> Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
>
> Hannes>     * ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
> Hannes>     * tui/tui-data.h (struct tui_win_info): Add click function.
> Hannes>     * tui/tui-io.c (tui_prep_terminal): Enable mouse events.
> Hannes>     (tui_deprep_terminal): Disable mouse events.
> Hannes>     (tui_dispatch_ctrl_char): Handle KEY_MOUSE.
> Hannes>     * tui/tui.c (tui_disable): Disable mouse events.
>
> Looks good.  Thank you again.

Pushed both, thanks.


Hannes

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 3:21 p.m., Hannes Domani via Gdb-patches wrote:
>  Am Freitag, 4. Juni 2021, 15:51:27 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> 
>>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>> Hannes> Implements an overridable tui_win_info::click method whose arguments
>> Hannes> are the mouse coordinates inside the specific window, and the mouse
>> Hannes> button clicked.
>>
>> Hannes> And if the curses implementation supports 5 buttons, the 4th and 5th
>> Hannes> buttons are used for scrolling.
>>
>> Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
>>
>> Hannes>     * ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
>> Hannes>     * tui/tui-data.h (struct tui_win_info): Add click function.
>> Hannes>     * tui/tui-io.c (tui_prep_terminal): Enable mouse events.
>> Hannes>     (tui_deprep_terminal): Disable mouse events.
>> Hannes>     (tui_dispatch_ctrl_char): Handle KEY_MOUSE.
>> Hannes>     * tui/tui.c (tui_disable): Disable mouse events.
>>
>> Looks good.  Thank you again.
> 
> Pushed both, thanks.
> 

Yay, mouse support finally.  Thank you!

Don't we need a NEWS entry, though?

Pedro Alves

Re: [PATCHv3 1/2] Initial TUI mouse support

[Hannes Domani]

 Am Freitag, 4. Juni 2021, 17:20:44 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

> On 2021-06-04 3:21 p.m., Hannes Domani via Gdb-patches wrote:
>
> >  Am Freitag, 4. Juni 2021, 15:51:27 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> >
> >>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
> >>
> >> Hannes> Implements an overridable tui_win_info::click method whose arguments
> >> Hannes> are the mouse coordinates inside the specific window, and the mouse
> >> Hannes> button clicked.
> >>
> >> Hannes> And if the curses implementation supports 5 buttons, the 4th and 5th
> >> Hannes> buttons are used for scrolling.
> >>
> >> Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
> >>
> >> Hannes>     * ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
> >> Hannes>     * tui/tui-data.h (struct tui_win_info): Add click function.
> >> Hannes>     * tui/tui-io.c (tui_prep_terminal): Enable mouse events.
> >> Hannes>     (tui_deprep_terminal): Disable mouse events.
> >> Hannes>     (tui_dispatch_ctrl_char): Handle KEY_MOUSE.
> >> Hannes>     * tui/tui.c (tui_disable): Disable mouse events.
> >>
> >> Looks good.  Thank you again.
> >
> > Pushed both, thanks.
>
> >
>
> Yay, mouse support finally.  Thank you!
>
> Don't we need a NEWS entry, though?

I never think of NEWS.

See at the end what I came up with, maybe the phrasing could need some
improvement.


Hannes


From 95c7890b45cbfb80682451962f5e57f3eef95a03 Mon Sep 17 00:00:00 2001
From: Hannes Domani <ssbssa@yahoo.de>
Date: Fri, 4 Jun 2021 17:58:15 +0200
Subject: [PATCH] Mention mouse support in NEWS

gdb/ChangeLog:

2021-06-04  Hannes Domani  <ssbssa@yahoo.de>

    * NEWS: Mention mouse support.
---
 gdb/NEWS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index ab678acec8b..8c876e71e63 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -75,6 +75,9 @@
   and "-eiex" that allow options (that would normally appear in a
   gdbearlyinit file) to be passed on the command line.
 
+* TUI windows now support mouse actions.  The mouse wheel scrolls the
+  appropriate window, and Python TUI windows can receive mouse click events.
+
 * New commands
 
 set debug event-loop
--
2.15.1.windows.2

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 5:06 p.m., Hannes Domani wrote:
>  Am Freitag, 4. Juni 2021, 17:20:44 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

>>
>> Don't we need a NEWS entry, though?
> 
> I never think of NEWS.
> 
> See at the end what I came up with, maybe the phrasing could need some
> improvement.
> 

> 
> From 95c7890b45cbfb80682451962f5e57f3eef95a03 Mon Sep 17 00:00:00 2001
> From: Hannes Domani <ssbssa@yahoo.de>
> Date: Fri, 4 Jun 2021 17:58:15 +0200
> Subject: [PATCH] Mention mouse support in NEWS
> 
> gdb/ChangeLog:
> 
> 2021-06-04  Hannes Domani  <ssbssa@yahoo.de>
> 
>     * NEWS: Mention mouse support.
> ---
>  gdb/NEWS | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index ab678acec8b..8c876e71e63 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -75,6 +75,9 @@
>    and "-eiex" that allow options (that would normally appear in a
>    gdbearlyinit file) to be passed on the command line.
>  
> +* TUI windows now support mouse actions.  The mouse wheel scrolls the
> +  appropriate window, and Python TUI windows can receive mouse click events.
> +

I think you should (also or in alternative) mention the Python bits in the "Python API" section.

It would be nice if we had a section in the manual for the TUI mouse too.  I suspect we'll end up
with some settings/knobs to configure mouse support, and that'd be the place to add them.

We can start small, IMO.  How about this?

From: Pedro Alves <pedro@palves.net>
Date: 2021-06-04 17:12:41 +0100

Document TUI mouse support in the manual

gdb/doc/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* gdb.texinfo (TUI): <TUI Mouse Support>: New node/section.

---

 gdb/doc/gdb.texinfo |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 90d827a50e7..486df2ec5f4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -28372,6 +28372,7 @@ enable} or @kbd{C-x C-a}.  @xref{TUI Commands, ,TUI Commands}, and
 * TUI Overview::                TUI overview
 * TUI Keys::                    TUI key bindings
 * TUI Single Key Mode::         TUI single key mode
+* TUI Mouse Support::           TUI mouse support
 * TUI Commands::                TUI-specific commands
 * TUI Configuration::           TUI configuration variables
 @end menu
@@ -28661,6 +28662,13 @@ If @value{GDBN} was built with Readline 8.0 or later, the TUI
 SingleKey keymap will be named @samp{SingleKey}.  This can be used in
 @file{.inputrc} to add additional bindings to this keymap.
 
+@node TUI Mouse Support
+@section TUI Mouse Support
+@cindex TUI mouse support
+
+The TUI supports mouse actions.  You can click on TUI windows to
+select them, and the mouse wheel scrolls the appropriate window.
+
 @node TUI Commands
 @section TUI-specific Commands
 @cindex TUI commands

Re: [PATCHv3 1/2] Initial TUI mouse support

[Eli Zaretskii]

> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 4 Jun 2021 17:23:50 +0100
> 
> +@node TUI Mouse Support
> +@section TUI Mouse Support
> +@cindex TUI mouse support
> +
> +The TUI supports mouse actions.  You can click on TUI windows to
> +select them, and the mouse wheel scrolls the appropriate window.

Thanks.  Like in NEWS, I think this should be qualified by mouse
support availability in the curses library in use.

Re: [PATCHv3 1/2] Initial TUI mouse support

[Eli Zaretskii]

> Date: Fri, 4 Jun 2021 16:06:03 +0000 (UTC)
> From: Hannes Domani via Gdb-patches <gdb-patches@sourceware.org>
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index ab678acec8b..8c876e71e63 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -75,6 +75,9 @@
>    and "-eiex" that allow options (that would normally appear in a
>    gdbearlyinit file) to be passed on the command line.
>  
> +* TUI windows now support mouse actions.  The mouse wheel scrolls the
> +  appropriate window, and Python TUI windows can receive mouse click events.
> +

Thanks.  The text is okay, but maybe we should qualify it by saying
something like "if the curses library supports the mouse".

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 4:20 p.m., Pedro Alves wrote:
> On 2021-06-04 3:21 p.m., Hannes Domani via Gdb-patches wrote:
>>  Am Freitag, 4. Juni 2021, 15:51:27 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
>>
>>>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>
>>> Hannes> Implements an overridable tui_win_info::click method whose arguments
>>> Hannes> are the mouse coordinates inside the specific window, and the mouse
>>> Hannes> button clicked.
>>>
>>> Hannes> And if the curses implementation supports 5 buttons, the 4th and 5th
>>> Hannes> buttons are used for scrolling.
>>>
>>> Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
>>>
>>> Hannes>     * ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
>>> Hannes>     * tui/tui-data.h (struct tui_win_info): Add click function.
>>> Hannes>     * tui/tui-io.c (tui_prep_terminal): Enable mouse events.
>>> Hannes>     (tui_deprep_terminal): Disable mouse events.
>>> Hannes>     (tui_dispatch_ctrl_char): Handle KEY_MOUSE.
>>> Hannes>     * tui/tui.c (tui_disable): Disable mouse events.
>>>
>>> Looks good.  Thank you again.
>>
>> Pushed both, thanks.
>>
> 
> Yay, mouse support finally.  Thank you!

Unfortunately, now that I try it, it's broken for me.  And it's broken in a very bad way -- I think
this should block the release or be disabled until we figure out what's wrong.  It definitely
makes GDB unusable for me.

The trouble is that now pressing anywhere on the screen with the mouse just results in
weird characters being printed on the command line window (probably uninterpreted control
sequences).  That even prevents me from selecting text (something I do often) -- I wanted to do
that to paste the results here.  I even tried suspending GDB with ^Z to then copy the text, but
that still leaves the mouse messed up.  See this screenshot:

  https://i.imgur.com/bO7FKDO.png

This was on Ubuntu 20.04.

Pedro Alves

Re: [PATCHv3 1/2] Initial TUI mouse support

[Hannes Domani]

 Am Freitag, 4. Juni 2021, 18:29:52 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

> On 2021-06-04 4:20 p.m., Pedro Alves wrote:
> > On 2021-06-04 3:21 p.m., Hannes Domani via Gdb-patches wrote:
> >>  Am Freitag, 4. Juni 2021, 15:51:27 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> >>
> >>>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
> >>>
> >>> Hannes> Implements an overridable tui_win_info::click method whose arguments
> >>> Hannes> are the mouse coordinates inside the specific window, and the mouse
> >>> Hannes> button clicked.
> >>>
> >>> Hannes> And if the curses implementation supports 5 buttons, the 4th and 5th
> >>> Hannes> buttons are used for scrolling.
> >>>
> >>> Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
> >>>
> >>> Hannes>     * ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
> >>> Hannes>     * tui/tui-data.h (struct tui_win_info): Add click function.
> >>> Hannes>     * tui/tui-io.c (tui_prep_terminal): Enable mouse events.
> >>> Hannes>     (tui_deprep_terminal): Disable mouse events.
> >>> Hannes>     (tui_dispatch_ctrl_char): Handle KEY_MOUSE.
> >>> Hannes>     * tui/tui.c (tui_disable): Disable mouse events.
> >>>
> >>> Looks good.  Thank you again.
> >>
> >> Pushed both, thanks.
> >>
> >
> > Yay, mouse support finally.  Thank you!
>
> Unfortunately, now that I try it, it's broken for me.  And it's broken in a very bad way -- I think
> this should block the release or be disabled until we figure out what's wrong.  It definitely
> makes GDB unusable for me.
>
> The trouble is that now pressing anywhere on the screen with the mouse just results in
> weird characters being printed on the command line window (probably uninterpreted control
> sequences).  That even prevents me from selecting text (something I do often) -- I wanted to do
> that to paste the results here.  I even tried suspending GDB with ^Z to then copy the text, but
> that still leaves the mouse messed up.  See this screenshot:
>
>   https://i.imgur.com/bO7FKDO.png
>
> This was on Ubuntu 20.04.

OK, that's bad.
What's the procedure for this sort or problem? Reverting?

I didn't expect this kind of problem, it works fine for me on a remote Linux
via putty.


Hannes

Re: [PATCHv3 1/2] Initial TUI mouse support

[Joel Brobecker]

Hi Pedro and Hannes,

Thanks Pedro for having reported this regression. I agree we can't
branch with this sort of issue!

On Fri, Jun 04, 2021 at 04:48:14PM +0000, Hannes Domani wrote:
>  Am Freitag, 4. Juni 2021, 18:29:52 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:
> 
> > On 2021-06-04 4:20 p.m., Pedro Alves wrote:
> > > On 2021-06-04 3:21 p.m., Hannes Domani via Gdb-patches wrote:
> > >>  Am Freitag, 4. Juni 2021, 15:51:27 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> > >>
> > >>>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
> > >>>
> > >>> Hannes> Implements an overridable tui_win_info::click method whose arguments
> > >>> Hannes> are the mouse coordinates inside the specific window, and the mouse
> > >>> Hannes> button clicked.
> > >>>
> > >>> Hannes> And if the curses implementation supports 5 buttons, the 4th and 5th
> > >>> Hannes> buttons are used for scrolling.
> > >>>
> > >>> Hannes> 2021-06-03  Hannes Domani  <ssbssa@yahoo.de>
> > >>>
> > >>> Hannes>     * ser-mingw.c (console_select_thread): Handle MOUSE_EVENT.
> > >>> Hannes>     * tui/tui-data.h (struct tui_win_info): Add click function.
> > >>> Hannes>     * tui/tui-io.c (tui_prep_terminal): Enable mouse events.
> > >>> Hannes>     (tui_deprep_terminal): Disable mouse events.
> > >>> Hannes>     (tui_dispatch_ctrl_char): Handle KEY_MOUSE.
> > >>> Hannes>     * tui/tui.c (tui_disable): Disable mouse events.
> > >>>
> > >>> Looks good.  Thank you again.
> > >>
> > >> Pushed both, thanks.
> > >>
> > >
> > > Yay, mouse support finally.  Thank you!
> >
> > Unfortunately, now that I try it, it's broken for me.  And it's broken in a very bad way -- I think
> > this should block the release or be disabled until we figure out what's wrong.  It definitely
> > makes GDB unusable for me.
> >
> > The trouble is that now pressing anywhere on the screen with the mouse just results in
> > weird characters being printed on the command line window (probably uninterpreted control
> > sequences).  That even prevents me from selecting text (something I do often) -- I wanted to do
> > that to paste the results here.  I even tried suspending GDB with ^Z to then copy the text, but
> > that still leaves the mouse messed up.  See this screenshot:
> >
> >   https://i.imgur.com/bO7FKDO.png
> >
> > This was on Ubuntu 20.04.
> 
> OK, that's bad.
> What's the procedure for this sort or problem? Reverting?
> 
> I didn't expect this kind of problem, it works fine for me on a remote Linux
> via putty.

If you can reproduce, understand, and fix quickly, we can go with that.
Otherwise, for an issue with this kind of effect, we indeed usually start
by reverting, or at least disabling the code entirely when that's easily
done.

-- 
Joel

Re: [PATCHv3 1/2] Initial TUI mouse support

[Simon Marchi]

On 2021-06-04 12:29 p.m., Pedro Alves wrote:
> The trouble is that now pressing anywhere on the screen with the mouse just results in
> weird characters being printed on the command line window (probably uninterpreted control
> sequences).  That even prevents me from selecting text (something I do often) -- I wanted to do
> that to paste the results here.  I even tried suspending GDB with ^Z to then copy the text, but
> that still leaves the mouse messed up.  See this screenshot:
> 
>   https://i.imgur.com/bO7FKDO.png
> 
> This was on Ubuntu 20.04.

Obviously the printing random characters is bad.  But it's possible that
the not being able to select text is normal, as the application (GDB)
now supports mouse events.  In some terminal emulators, you can press
shift while you click to say "I really want to select the display
characters, not send mouse events to the program".

It's the same with other TUI apps for me:

 - tmux (with "set -g mouse on" in ~/.tmux.conf)
 - tig (I don't remember if anything is needed to enable mouse support)
 - weechat (I don't remember if anything is needed to enable mouse
   support)

In some of these programs, mouse support is disabled by default and you
have to use some setting to enable it.  So by default, standard
selection would work.  But if you enable mouse suport, then you have to
use shift+click to select.

Simon

Re: [PATCHv3 1/2] Initial TUI mouse support

[Joel Brobecker]

> Obviously the printing random characters is bad.  But it's possible that
> the not being able to select text is normal, as the application (GDB)
> now supports mouse events.  In some terminal emulators, you can press
> shift while you click to say "I really want to select the display
> characters, not send mouse events to the program".
> 
> It's the same with other TUI apps for me:
> 
>  - tmux (with "set -g mouse on" in ~/.tmux.conf)
>  - tig (I don't remember if anything is needed to enable mouse support)
>  - weechat (I don't remember if anything is needed to enable mouse
>    support)

FWIW, "vim" behaves like that too.

That being said, if we go that route, we should probably have that
mentioned in the manual and in the NEWS file.

-- 
Joel

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 7:13 p.m., Simon Marchi wrote:
> On 2021-06-04 12:29 p.m., Pedro Alves wrote:
>> The trouble is that now pressing anywhere on the screen with the mouse just results in
>> weird characters being printed on the command line window (probably uninterpreted control
>> sequences).  That even prevents me from selecting text (something I do often) -- I wanted to do
>> that to paste the results here.  I even tried suspending GDB with ^Z to then copy the text, but
>> that still leaves the mouse messed up.  See this screenshot:
>>
>>   https://i.imgur.com/bO7FKDO.png
>>
>> This was on Ubuntu 20.04.
> 
> Obviously the printing random characters is bad.  But it's possible that
> the not being able to select text is normal, as the application (GDB)
> now supports mouse events.  In some terminal emulators, you can press
> shift while you click to say "I really want to select the display
> characters, not send mouse events to the program".

I'm on my Fedora 32 laptop currently, and here it works fine.

You can select text without having press any key.  It basically works
as before.  I suppose we could teach GDB to be smarter about text selection -- like,
if you select several lines of source code in the TUI source window, ideally
we'd select just the source code, instead of the source code plus the window
borders, etc.  Then we would have an "escape" key to be able to disable special
mouse mode so you can select text in the whole terminal using regular text
terminal selection, and I suppose that that's what "shift" does in those programs
you mention.

I'm surprised to find that clicking a window doesn't set focus on it, though.
Is that supposed to work?

Pedro Alves

> 
> It's the same with other TUI apps for me:
> 
>  - tmux (with "set -g mouse on" in ~/.tmux.conf)
>  - tig (I don't remember if anything is needed to enable mouse support)
>  - weechat (I don't remember if anything is needed to enable mouse
>    support)
> 
> In some of these programs, mouse support is disabled by default and you
> have to use some setting to enable it.  So by default, standard
> selection would work.  But if you enable mouse suport, then you have to
> use shift+click to select.
> 
> Simon
> 

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 9:31 p.m., Pedro Alves wrote:
> On 2021-06-04 7:13 p.m., Simon Marchi wrote:
>> On 2021-06-04 12:29 p.m., Pedro Alves wrote:
>>> The trouble is that now pressing anywhere on the screen with the mouse just results in
>>> weird characters being printed on the command line window (probably uninterpreted control
>>> sequences).  That even prevents me from selecting text (something I do often) -- I wanted to do
>>> that to paste the results here.  I even tried suspending GDB with ^Z to then copy the text, but
>>> that still leaves the mouse messed up.  See this screenshot:
>>>
>>>   https://i.imgur.com/bO7FKDO.png
>>>
>>> This was on Ubuntu 20.04.
>>
>> Obviously the printing random characters is bad.  But it's possible that
>> the not being able to select text is normal, as the application (GDB)
>> now supports mouse events.  In some terminal emulators, you can press
>> shift while you click to say "I really want to select the display
>> characters, not send mouse events to the program".
> 
> I'm on my Fedora 32 laptop currently, and here it works fine.
> 
> You can select text without having press any key.  It basically works
> as before.  

Huh, now that I pull+rebuild GDB, I need to press shift to select.  And now I'm not
sure what I tested anymore.  I could definitely scroll the TUI windows before
this latest rebuild.  Mistyfying...

Pedro Alves

> I suppose we could teach GDB to be smarter about text selection -- like,
> if you select several lines of source code in the TUI source window, ideally
> we'd select just the source code, instead of the source code plus the window
> borders, etc.  Then we would have an "escape" key to be able to disable special
> mouse mode so you can select text in the whole terminal using regular text
> terminal selection, and I suppose that that's what "shift" does in those programs
> you mention.
> 
> I'm surprised to find that clicking a window doesn't set focus on it, though.
> Is that supposed to work?
> 
> Pedro Alves
> 
>>
>> It's the same with other TUI apps for me:
>>
>>  - tmux (with "set -g mouse on" in ~/.tmux.conf)
>>  - tig (I don't remember if anything is needed to enable mouse support)
>>  - weechat (I don't remember if anything is needed to enable mouse
>>    support)
>>
>> In some of these programs, mouse support is disabled by default and you
>> have to use some setting to enable it.  So by default, standard
>> selection would work.  But if you enable mouse suport, then you have to
>> use shift+click to select.
>>
>> Simon
>>
> 

Re: [PATCHv3 1/2] Initial TUI mouse support

[Hannes Domani]

 Am Freitag, 4. Juni 2021, 22:31:31 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

> On 2021-06-04 7:13 p.m., Simon Marchi wrote:
> > On 2021-06-04 12:29 p.m., Pedro Alves wrote:
> >> The trouble is that now pressing anywhere on the screen with the mouse just results in
> >> weird characters being printed on the command line window (probably uninterpreted control
> >> sequences).  That even prevents me from selecting text (something I do often) -- I wanted to do
> >> that to paste the results here.  I even tried suspending GDB with ^Z to then copy the text, but
> >> that still leaves the mouse messed up.  See this screenshot:
> >>
> >>  https://i.imgur.com/bO7FKDO.png
> >>
> >> This was on Ubuntu 20.04.
> >
> > Obviously the printing random characters is bad.  But it's possible that
> > the not being able to select text is normal, as the application (GDB)
> > now supports mouse events.  In some terminal emulators, you can press
> > shift while you click to say "I really want to select the display
> > characters, not send mouse events to the program".
>
> I'm on my Fedora 32 laptop currently, and here it works fine.
>
> You can select text without having press any key.  It basically works
> as before.  I suppose we could teach GDB to be smarter about text selection -- like,
> if you select several lines of source code in the TUI source window, ideally
> we'd select just the source code, instead of the source code plus the window
> borders, etc.  Then we would have an "escape" key to be able to disable special
> mouse mode so you can select text in the whole terminal using regular text
> terminal selection, and I suppose that that's what "shift" does in those programs
> you mention.
>
> I'm surprised to find that clicking a window doesn't set focus on it, though.
> Is that supposed to work?

No, that's not implemented.
I was thinking about it, but wasn't sure what people prefer.
So I just kept the simpler variant.


Hannes

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 10:15 p.m., Hannes Domani wrote:
>  Am Freitag, 4. Juni 2021, 22:31:31 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

>> I'm surprised to find that clicking a window doesn't set focus on it, though.
>> Is that supposed to work?
> 
> No, that's not implemented.
> I was thinking about it, but wasn't sure what people prefer.
> So I just kept the simpler variant.

I would prefer being able to click to select.  Clicking things and nothing happening
seems like a waste of actions.  :-)  If different people prefer different things,
that calls for a setting/option.  Maybe some people might prefer focus-follow-mouse?  (not me.)

Pedro Alves

Re: [PATCHv3 1/2] Initial TUI mouse support

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> I suppose we could teach GDB to be smarter about text selection -- like,
Pedro> if you select several lines of source code in the TUI source window, ideally
Pedro> we'd select just the source code, instead of the source code plus the window
Pedro> borders, etc.

Is it possible to implement this?  I looked a little but didn't see a
way to set the selection.  Hopefully I'm just looking for the wrong
keywords...

Tom

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-10 7:44 p.m., Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> I suppose we could teach GDB to be smarter about text selection -- like,
> Pedro> if you select several lines of source code in the TUI source window, ideally
> Pedro> we'd select just the source code, instead of the source code plus the window
> Pedro> borders, etc.
> 
> Is it possible to implement this?  I looked a little but didn't see a
> way to set the selection.  Hopefully I'm just looking for the wrong
> keywords...

Sorry, I just assumed it was possible, but I don't really know.

Pedro Alves

Re: [PATCHv3 1/2] Initial TUI mouse support

[Tom Tromey]

>> Is it possible to implement this?  I looked a little but didn't see a
>> way to set the selection.  Hopefully I'm just looking for the wrong
>> keywords...

Pedro> Sorry, I just assumed it was possible, but I don't really know.

By coincidence I learned that this is indeed possible.
It's called "OSC 52":

https://www.reddit.com/r/vim/comments/k1ydpn/a_guide_on_how_to_copy_text_from_anywhere/

And if you search for "emacs" here, there's some information as well:

https://invisible-island.net/xterm/ctlseqs/ctlseqs.html

Tom

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-18 4:01 p.m., Tom Tromey wrote:
>>> Is it possible to implement this?  I looked a little but didn't see a
>>> way to set the selection.  Hopefully I'm just looking for the wrong
>>> keywords...
> 
> Pedro> Sorry, I just assumed it was possible, but I don't really know.
> 
> By coincidence I learned that this is indeed possible.
> It's called "OSC 52":
> 
> https://www.reddit.com/r/vim/comments/k1ydpn/a_guide_on_how_to_copy_text_from_anywhere/
> 
> And if you search for "emacs" here, there's some information as well:
> 
> https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
> 

That's awesome!  Great find.

Re: [PATCHv3 1/2] Initial TUI mouse support

[Tom Tromey]

Pedro> The trouble is that now pressing anywhere on the screen with the
Pedro> mouse just results in weird characters being printed on the
Pedro> command line window (probably uninterpreted control sequences).

I tried it here, and it worked fine for me.

I couldn't select text, but I didn't see any control characters.

Pedro> This was on Ubuntu 20.04.

I'm on Fedora 32.

I wonder if there's something weird about the Ubuntu build of ncurses.
Or the Fedora one.

Tom

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 7:46 p.m., Tom Tromey wrote:
> Pedro> The trouble is that now pressing anywhere on the screen with the
> Pedro> mouse just results in weird characters being printed on the
> Pedro> command line window (probably uninterpreted control sequences).
> 
> I tried it here, and it worked fine for me.
> 
> I couldn't select text, but I didn't see any control characters.
> 
> Pedro> This was on Ubuntu 20.04.
> 
> I'm on Fedora 32.

I'm currently on Fedora 32 too, and while it works here too, I just found a way to reproduce a similar
problem that I'm seeing on the Ubuntu machine (which I currently don't have access to).  (Though
on Ubuntu it was worse.)

Try this:

$ ./gdb ./gdb
...
(gdb) start
c-x a
scroll up and down with mouse to check that it works ok.
c-x o # select command window
scroll up and down with mouse, and see escape codes being printed on the command window

If I "c-x o" again to select the source window, then scrolling works correctly again.

If I "c-x o" again to select the command window, then scrolling and clicking with the mouse 
prints more bad characters.  Like these:

(top-gdb) 64;40;21M64;40;21M65;40;21M65;40;21M64;40;21M64;40;21M64;40;21M65;40;21M0;67;32M0;67;32m0;64;18M0;64;18m0;64;18M0;64;18m2;64;18M2;64;18m0;64;18M0;64;18M0;64;18M0
;64;18m1;64;18M1;64;18m64;64;18M64;64;18M1;64;19M1;64;19m1;64;19M1;64;19m2;64;19M2;64;19m0;64;19M0;64;19m0;51;13M0;51;13m0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0
;1;27M0;1;27M0;1;27m

Pedro Alves

> 
> I wonder if there's something weird about the Ubuntu build of ncurses.
> Or the Fedora one.
> 
> Tom
> 

Re: [PATCHv3 1/2] Initial TUI mouse support

[Pedro Alves]

On 2021-06-04 9:54 p.m., Pedro Alves wrote:
> On 2021-06-04 7:46 p.m., Tom Tromey wrote:
>> Pedro> The trouble is that now pressing anywhere on the screen with the
>> Pedro> mouse just results in weird characters being printed on the
>> Pedro> command line window (probably uninterpreted control sequences).
>>
>> I tried it here, and it worked fine for me.
>>
>> I couldn't select text, but I didn't see any control characters.
>>
>> Pedro> This was on Ubuntu 20.04.
>>
>> I'm on Fedora 32.
> 
> I'm currently on Fedora 32 too, and while it works here too, I just found a way to reproduce a similar
> problem that I'm seeing on the Ubuntu machine (which I currently don't have access to).  (Though
> on Ubuntu it was worse.)
> 
> Try this:
> 
> $ ./gdb ./gdb
> ...
> (gdb) start
> c-x a
> scroll up and down with mouse to check that it works ok.
> c-x o # select command window
> scroll up and down with mouse, and see escape codes being printed on the command window
> 
> If I "c-x o" again to select the source window, then scrolling works correctly again.
> 
> If I "c-x o" again to select the command window, then scrolling and clicking with the mouse 
> prints more bad characters.  Like these:
> 
> (top-gdb) 64;40;21M64;40;21M65;40;21M65;40;21M64;40;21M64;40;21M64;40;21M65;40;21M0;67;32M0;67;32m0;64;18M0;64;18m0;64;18M0;64;18m2;64;18M2;64;18m0;64;18M0;64;18M0;64;18M0
> ;64;18m1;64;18M1;64;18m64;64;18M64;64;18M1;64;19M1;64;19m1;64;19M1;64;19m2;64;19M2;64;19m0;64;19M0;64;19m0;51;13M0;51;13m0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0
> ;1;27M0;1;27M0;1;27m

I have access to the Ubuntu machine again now.  The symptom I see now the same as on Fedora -- I can scroll
the source window, when when I focus on the command window, I start seeing all the badness.  I don't think
I focused on the command window earlier when I first reported it, maybe there's some other way to trigger
it, but can't say for sure.  So it is looking like this isn't OS specific afterall.

Sorry for the spotty reporting.  Hopefully the reproducer helps identifying the issue.

Pedro Alves

Re: [PATCHv3 1/2] Initial TUI mouse support

[Hannes Domani]

 Am Samstag, 5. Juni 2021, 01:48:38 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

> On 2021-06-04 9:54 p.m., Pedro Alves wrote:
> > On 2021-06-04 7:46 p.m., Tom Tromey wrote:
> >> Pedro> The trouble is that now pressing anywhere on the screen with the
> >> Pedro> mouse just results in weird characters being printed on the
> >> Pedro> command line window (probably uninterpreted control sequences).
> >>
> >> I tried it here, and it worked fine for me.
> >>
> >> I couldn't select text, but I didn't see any control characters.
> >>
> >> Pedro> This was on Ubuntu 20.04.
> >>
> >> I'm on Fedora 32.
> >
> > I'm currently on Fedora 32 too, and while it works here too, I just found a way to reproduce a similar
> > problem that I'm seeing on the Ubuntu machine (which I currently don't have access to).  (Though
> > on Ubuntu it was worse.)
> >
> > Try this:
> >
> > $ ./gdb ./gdb
> > ...
> > (gdb) start
> > c-x a
> > scroll up and down with mouse to check that it works ok.
> > c-x o # select command window
> > scroll up and down with mouse, and see escape codes being printed on the command window
> >
> > If I "c-x o" again to select the source window, then scrolling works correctly again.
> >
> > If I "c-x o" again to select the command window, then scrolling and clicking with the mouse
> > prints more bad characters.  Like these:
> >
> > (top-gdb) 64;40;21M64;40;21M65;40;21M65;40;21M64;40;21M64;40;21M64;40;21M65;40;21M0;67;32M0;67;32m0;64;18M0;64;18m0;64;18M0;64;18m2;64;18M2;64;18m0;64;18M0;64;18M0;64;18M0
> > ;64;18m1;64;18M1;64;18m64;64;18M64;64;18M1;64;19M1;64;19m1;64;19M1;64;19m2;64;19M2;64;19m0;64;19M0;64;19m0;51;13M0;51;13m0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0;1;27M0
> > ;1;27M0;1;27M0;1;27m
>
> I have access to the Ubuntu machine again now.  The symptom I see now the same as on Fedora -- I can scroll
> the source window, when when I focus on the command window, I start seeing all the badness.  I don't think
> I focused on the command window earlier when I first reported it, maybe there's some other way to trigger
> it, but can't say for sure.  So it is looking like this isn't OS specific afterall.
>
> Sorry for the spotty reporting.  Hopefully the reproducer helps identifying the issue.

On Windows I can mostly reproduce this, when the command window has focus,
mouse scrolling just doesn't work.
I don't see any escape sequences, because the Windows console works different.
I probably wouldn't have noticed this for a long time, because I never set
the focus to the command window (I always wondered why this is possible at all).

Anyways, I think this happens because when the command window gets focus,
we disable the curses keypad.
As the docu says:

  The keypad option enables the keypad of the user's terminal. If enabled
  (bf is TRUE), the user can press a function key (such as an arrow key) and
  wgetch returns a single value representing the function key, as in KEY_LEFT.
  If disabled (bf is FALSE), curses does not treat function keys specially
  and the program has to interpret the escape sequences itself.

I imagine the same is true for mouse escape sequences.
So I would just disable the mouse when the command window has focus, but
what do you think?


Hannes

Re: [PATCHv3 1/2] Initial TUI mouse support

[Eli Zaretskii]

> Date: Sat, 5 Jun 2021 14:40:17 +0000 (UTC)
> From: Hannes Domani via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Joel Brobecker <brobecker@adacore.com>,
>  Hannes Domani via Gdb-patches <gdb-patches@sourceware.org>
> 
> So I would just disable the mouse when the command window has focus, but
> what do you think?

If we end up doing that (and I have no objections, FWIW), let's
reflect that in the manual.

Thanks.

Re: [PATCHv3 1/2] Initial TUI mouse support

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> I imagine the same is true for mouse escape sequences.
Hannes> So I would just disable the mouse when the command window has focus, but
Hannes> what do you think?

I think this is a good fallback to have, but it would be better if we
could make it really work somehow.  The problem with doing this is that
if the command window has focus, then things like clicking in the source
to set a breakpoint will not work.

Tom

Re: [PATCHv3 1/2] Initial TUI mouse support

[Hannes Domani]

 Am Donnerstag, 10. Juni 2021, 20:47:01 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Hannes> I imagine the same is true for mouse escape sequences.
> Hannes> So I would just disable the mouse when the command window has focus, but
> Hannes> what do you think?
>
> I think this is a good fallback to have, but it would be better if we
> could make it really work somehow.  The problem with doing this is that
> if the command window has focus, then things like clicking in the source
> to set a breakpoint will not work.

That would probably mean that keypad also has to be enabled when the
command window has focus.
I did a short test where I tried this, but then gdb crashed somewhere in
readline, and I stopped there, because I didn't want to debug readline code.


Hannes

POC: Make the TUI command window support the mouse (Re: [PATCHv3 1/2] Initial TUI mouse support)

[Pedro Alves]

On 2021-06-11 12:02 p.m., Hannes Domani wrote:
>  Am Donnerstag, 10. Juni 2021, 20:47:01 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> 
>>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>> Hannes> I imagine the same is true for mouse escape sequences.
>> Hannes> So I would just disable the mouse when the command window has focus, but
>> Hannes> what do you think?
>>
>> I think this is a good fallback to have, but it would be better if we
>> could make it really work somehow.  The problem with doing this is that
>> if the command window has focus, then things like clicking in the source
>> to set a breakpoint will not work.
> 
> That would probably mean that keypad also has to be enabled when the
> command window has focus.
> I did a short test where I tried this, but then gdb crashed somewhere in
> readline, and I stopped there, because I didn't want to debug readline code.

We disable the keypad in the command window because we want to pass all escape
sequences to readline, unprocessed by ncurses, so that e.g., up/down navigates
the command history, left/right moves the cursor position, etc., just like if
you weren't in the TUI.

I first thought of fixing this by enabling the keypad, and then where we handle
KEY_UP, etc., special case the command window, and call readline functions that do
what you'd expect.  But the thing is that ends up being a poor readline emulation,
and I didn't like where this was going.  Especially when I realized that for example,
when you're in the middle for a reverse-search (c-r), KEY_UP does something different.
Etc.

I had a different idea to make this all work.  Warning, this is either a brilliant hack,
or a really nasty hack, depending on perspective.  :-)

The idea is to keep the keypad disabled in the command window.  However,
we need to enable the keypad for ncurses to process mouse escape sequences.
This is a conflict.  So here's the main trick behind the idea.  It breaks the conflict.
Create a separate ncurses screen/terminal, with newterm, which reads from a pipe instead
of from stdin.  Then, flush data from stdin into this pipe, BUT, make sure to never flush
a non-mouse escape sequence into the pipe.  It's like the pipe is stdin, but with any
non-mouse escape sequence filtered out.  This way, ncurses only either sees mouse
escape sequences or normal ASCII keys.  In order to do this, we need to use
a separate thread to flush stdin to the pipe.

I gave this a try the other weekend, and ran into issues.  Today/tonight I gave it
another go, and got it working.  It is a bit rough around the edges, but
seems to work nicely.

From a3bf7056c7dee837816807d41d33abda45cfd51d Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Sat, 5 Jun 2021 19:11:09 +0100
Subject: [PATCH] Make the TUI command window support the mouse

Change-Id: Ie0a7d849943cfb47f4a6589e1c73341563740fa9
---
 gdb/tui/tui-io.c | 549 ++++++++++++++++++++++++++++++++++++++++++++---
 gdb/tui/tui-io.h |   3 +
 gdb/tui/tui.c    |   4 +
 3 files changed, 527 insertions(+), 29 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 7df0e2f1bd3..6c1c3f2f5f0 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -946,6 +946,34 @@ tui_initialize_io (void)
 #endif
 }
 
+/* Dispatch the correct tui function based upon the mouse event.  */
+
+static void
+tui_dispatch_mouse_event (const MEVENT &mev)
+{
+  for (tui_win_info *wi : all_tui_windows ())
+    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
+	&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
+      {
+	if ((mev.bstate & BUTTON1_CLICKED) != 0
+	    || (mev.bstate & BUTTON2_CLICKED) != 0
+	    || (mev.bstate & BUTTON3_CLICKED) != 0)
+	  {
+	    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
+	      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
+			 : 3);
+	    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
+	  }
+#ifdef BUTTON5_PRESSED
+	else if ((mev.bstate & BUTTON4_PRESSED) != 0)
+	  wi->backward_scroll (3);
+	else if ((mev.bstate & BUTTON5_PRESSED) != 0)
+	  wi->forward_scroll (3);
+#endif
+	break;
+      }
+}
+
 /* Dispatch the correct tui function based upon the control
    character.  */
 static unsigned int
@@ -959,7 +987,9 @@ tui_dispatch_ctrl_char (unsigned int ch)
 
   /* If no window has the focus, or if the focus window can't scroll,
      just pass the character through.  */
-  if (win_info == NULL || !win_info->can_scroll ())
+  if (win_info == NULL
+      || (win_info != TUI_CMD_WIN
+	  && !win_info->can_scroll ()))
     return ch;
 
   switch (ch)
@@ -988,30 +1018,8 @@ tui_dispatch_ctrl_char (unsigned int ch)
     case KEY_MOUSE:
 	{
 	  MEVENT mev;
-	  if (getmouse (&mev) != OK)
-	    break;
-
-	  for (tui_win_info *wi : all_tui_windows ())
-	    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
-		&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
-	      {
-		if ((mev.bstate & BUTTON1_CLICKED) != 0
-		    || (mev.bstate & BUTTON2_CLICKED) != 0
-		    || (mev.bstate & BUTTON3_CLICKED) != 0)
-		  {
-		    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
-		      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
-				 : 3);
-		    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
-		  }
-#ifdef BUTTON5_PRESSED
-		else if ((mev.bstate & BUTTON4_PRESSED) != 0)
-		  wi->backward_scroll (3);
-		else if ((mev.bstate & BUTTON5_PRESSED) != 0)
-		  wi->forward_scroll (3);
-#endif
-		break;
-	      }
+	  if (getmouse (&mev) == OK)
+	    tui_dispatch_mouse_event (mev);
 	}
       break;
 #endif
@@ -1067,6 +1075,251 @@ tui_inject_newline_into_command_window ()
     }
 }
 
+/* True if the mouse thread is the one reading stdin.  */
+static volatile bool mouse_thread_stdin = false;
+static volatile bool mouse_thread_stdin_out = false;
+
+/* The handle to the mouse thread.  */
+static pthread_t mouse_thread;
+
+/* Pipe used to wake up / interrupt the mouse thread.  */
+static int intr_mouse_thread[2];
+
+/* Pipe used to feed input data into the magic mouse screen.  */
+static int mouse_input_pipe[2];
+
+/* Data we've read from stdin but did not want passed to ncurses.  */
+static std::vector<int> mouse_chars;
+
+/* True if the next tut_getc_1 should drain from CHARS.  */
+static bool drain_mouse_chars;
+
+/* Enable debugging the mouse screen and thread processing.  */
+static bool debug_mouse_thread = 0;
+
+/* The escape sequence header for a mouse event.  */
+static const unsigned char mouse_seq[] = { 27, '[', 'M' };
+/* Pointer to the next expected character in a mouse escape sequence.
+   If we haven't seen any character of the sequence yet, this points
+   to the first character of MOUSE_SEQ.  If we've seen the whole
+   sequence header, this points to one-past-last of MOUSE_SEQ.  */
+static const unsigned char *mouse_seq_pos = nullptr;
+
+/* True if the last key we got out of gdb_wgetch was a KEY_MOUSE.  In
+   that case, continue reading chars using the magic mouse screen.  */
+static bool last_key_was_mouse = false;
+
+/* The mouse screen used to read mouse keys.  This screen's main
+   window has the keypad enabled so that it processes mouse escape
+   sequences.  But instead of reading from stdin, it reads from a
+   pipe.  This pipe is filled with data read from stdin, but filtered
+   -- it won't ever include non-mouse escape sequences.  This means
+   that reading from this curses screen either returns KEY_MOUSE, or
+   regular ASCII key, never KEY_UP, etc.  */
+static SCREEN *mouse_screen;
+
+/* Write CH to the mouse input pipe.  */
+
+static void
+write_mouse_pipe (int ch)
+{
+  unsigned char buf = ch;
+  write (mouse_input_pipe[1], &buf, 1);
+}
+
+/* Wake up the mouse thread.  */
+
+static void
+wake_mouse_thread ()
+{
+  unsigned char buf = '+';
+  write (intr_mouse_thread[1], &buf, 1);
+}
+
+/* Flush the INTR_MOUSE_THREAD pipe.  */
+
+static void
+flush_intr_mouse_pipe ()
+{
+  int ret;
+
+  do
+    {
+      char buf;
+      ret = read (intr_mouse_thread[0], &buf, 1);
+    }
+  while (ret >= 0 || (ret == -1 && errno == EINTR));
+}
+
+/* Flush data out of stdin, into the MOUSE_INPUT_PIPE.  Except, if we
+   see an escape sequence that isn't a mouse escape sequence, don't
+   put it in the pipe.  The idea is that we never want ncurses to see
+   non-mouse escape sequences.  We want to pass those directly to
+   readline.  E.g., we don't want ncurses to return KEY_UP, we want
+   readline instead to see the "key up" escape sequence, and call
+   "previous-history".  */
+
+static void *
+mouse_thread_fn (void *)
+{
+  /* XXXX: Block signals in parent before spawning this.  */
+
+  while (1)
+    {
+      fd_set readfds;
+
+      FD_ZERO (&readfds);
+      FD_SET (intr_mouse_thread[0], &readfds);
+
+      if (mouse_thread_stdin)
+	FD_SET (0, &readfds);
+
+      if (select (intr_mouse_thread[0] + 1, &readfds, NULL, NULL, NULL) == -1)
+	{
+	  if (errno == EINTR)
+	    continue;
+
+	  /* XXX: shouldn't throw...  */
+	  perror_with_name (("select"));
+	}
+
+      if (FD_ISSET (0, &readfds))
+	{
+	  unsigned char ch;
+	  int n = read (0, &ch, 1);
+	  if (n == 1)
+	    {
+	      if (ch == *mouse_seq_pos)
+		{
+		  /* Looks like another character part of a mouse
+		     escape sequence.  */
+		  mouse_seq_pos++;
+
+		  if (mouse_seq_pos == mouse_seq + sizeof (mouse_seq))
+		    {
+		      /* Yup, we saw "ESC [ M".  */
+
+		      if (debug_mouse_thread)
+			fprintf (stderr, "thread: got mouse sequence header\n");
+
+		      for (size_t i = 0; i < sizeof (mouse_seq); i++)
+			{
+			  write_mouse_pipe (mouse_seq[i]);
+			  if (debug_mouse_thread)
+			    fprintf (stderr, "thread: %c (%d) ",
+				     mouse_seq[i], mouse_seq[i]);
+			}
+
+		      mouse_seq_pos = mouse_seq;
+		    }
+		}
+	      else if (mouse_seq_pos > mouse_seq)
+		{
+		  if (debug_mouse_thread)
+		    fprintf (stderr,
+			     "thread: non-mouse escape sequence, STOP!\n");
+
+		  /* Store the sequence in MOUSE_CHARS instead of in
+		     the pipe.  We don't want ncurses to see it.  */
+		  for (size_t i = 0; i < mouse_seq_pos - mouse_seq; i++)
+		    {
+		      mouse_chars.push_back (mouse_seq[i]);
+		      if (debug_mouse_thread)
+			fprintf (stderr, "thread: %c (%d) ",
+				 mouse_seq[i], mouse_seq[i]);
+		    }
+		  mouse_chars.push_back (ch);
+		  if (debug_mouse_thread)
+		    fprintf (stderr, "thread: %c (%d) ", ch, ch);
+
+		  /* The mainline code is blocked in wgetch, and we
+		     need to wake it up, but we don't want to let
+		     ncurses see this sequence.  Solve this by putting
+		     a 0/NIL character in the pipe.  That pass through
+		     wgetch and we will end up returning 0 to
+		     readline, which readline interprets as "no op",
+		     exactly what we want.  Phew!  */
+		  write_mouse_pipe (0);
+
+		  /* Reset the sequence position pointer, and stop
+		     reading stdin until the mainline code tells us
+		     otherwise.  */
+		  mouse_seq_pos = mouse_seq;
+		  mouse_thread_stdin = false;
+		}
+	      else
+		{
+		  write_mouse_pipe (ch);
+
+		  if (debug_mouse_thread)
+		    fprintf (stderr, "thread: %c (%d) ", ch, ch);
+		}
+	    }
+	  else
+	    {
+	      if (debug_mouse_thread)
+		fprintf (stderr, "thread: => -1!");
+	    }
+	  if (debug_mouse_thread)
+	    fflush (stderr);
+	}
+
+      if (FD_ISSET (intr_mouse_thread[0], &readfds))
+	{
+	  int ret;
+
+	  if (debug_mouse_thread)
+	    fprintf (stderr, "break thread\n");
+
+	  do
+	    {
+	      char buf;
+	      ret = read (intr_mouse_thread[0], &buf, 1);
+	    }
+	  while (ret == -1 && errno == EINTR);
+
+	  mouse_thread_stdin_out = mouse_thread_stdin;
+	  continue;
+	}
+    }
+
+  return nullptr;
+}
+
+/* Initialize the special mouse screen, and all auxiliary bits.  */
+
+void
+init_mouse_screen ()
+{
+  /* Pseudo-stdin pipe for the ncurses mouse "terminal".  */
+  if (gdb_pipe_cloexec (mouse_input_pipe) != 0)
+    error (_("Cannot create pipe for mouse"));
+
+  /* The self-trick pipe used to wake up / interrupt the mouse
+     thread.  */
+  if (gdb_pipe_cloexec (intr_mouse_thread) != 0)
+    error (_("Cannot create pipe for mouse interruption"));
+  fcntl (intr_mouse_thread[0], F_SETFL, O_NONBLOCK);
+  fcntl (intr_mouse_thread[1], F_SETFL, O_NONBLOCK);
+
+  /* Create the mouse terminal.  It reads from the mouse input pipe,
+     and writes nowhere.  */
+  FILE *in = fdopen (mouse_input_pipe[0], "r");
+  FILE *out = fopen ("/dev/null", "w+");
+  setvbuf (in, NULL, _IONBF, BUFSIZ);
+
+  mouse_screen = newterm (NULL, out, in);
+  mousemask (ALL_MOUSE_EVENTS, NULL);
+
+  /* Enable the keypad, we want to use this terminal to process mouse
+     escape codes.  */
+  WINDOW *w = stdscr;
+  keypad (w, TRUE);
+
+  /* Spawn the mouse thread.  */
+  pthread_create (&mouse_thread, nullptr, mouse_thread_fn, nullptr);
+}
+
 /* Main worker for tui_getc.  Get a character from the command window.
    This is called from the readline package, but wrapped in a
    try/catch by tui_getc.  */
@@ -1084,14 +1337,251 @@ tui_getc_1 (FILE *fp)
   tui_readline_output (0, 0);
 #endif
 
-  ch = gdb_wgetch (w);
+  if (tui_win_with_focus () == TUI_CMD_WIN && current_ui->command_editing)
+    {
+      if (drain_mouse_chars)
+	{
+	  ch = mouse_chars.front ();
+	  mouse_chars.erase (mouse_chars.begin ());
+	  if (debug_mouse_thread)
+	    fprintf (stderr, "drain: got %c (%d), ", ch, ch);
+	  if (mouse_chars.empty ())
+	    {
+	      drain_mouse_chars = false;
+	      if (debug_mouse_thread)
+		fprintf (stderr, " : done\r\n");
+	    }
+	  else
+	    call_stdin_event_handler_again_p = 1;
+	  return ch;
+	}
+      else if (!last_key_was_mouse)
+	{
+	  ch = gdb_wgetch (w);
+	}
+
+      if (last_key_was_mouse || key_is_start_sequence (ch))
+	{
+	  /* Process this key sequence with the special mouse ncurses
+	     screen, which has the keypad enabled, and reads from
+	     MOUSE_INPUT_PIPE.  */
+
+	  /* Set the current screen to the mouse screen.  */
+	  WINDOW *prev_stdscr = stdscr;
+	  SCREEN *prev_s = set_term (mouse_screen);
+	  /* The mouse WINDOW.  */
+	  WINDOW *mw = stdscr;
+
+	  if (last_key_was_mouse)
+	    {
+	      if (debug_mouse_thread)
+		fprintf (stderr, "last key was mouse\r\n");
+	    }
+
+	  /* If we're just starting a sequence, block waiting for the
+	     whole sequence.  If we're here because we returned a
+	     mouse event before and we're now draining the curses
+	     buffer, disable blocking.  */
+	  if (last_key_was_mouse)
+	    nodelay (mw, TRUE);
+	  else
+	    nodelay (mw, FALSE);
+
+	  flush_intr_mouse_pipe ();
+
+	  /* If we're starting a new sequence, reset the sequence
+	     position.  If we last saw a mouse key, then the mouse
+	     thread may have read a partial mouse event out of stdin,
+	     so we need to let it continue the sequence where it left
+	     it last.  */
+	  if (!last_key_was_mouse)
+	    mouse_seq_pos = &mouse_seq[1];
+
+	  /* Set the thread reading from stdin.  */
+	  mouse_thread_stdin = true;
+	  wake_mouse_thread ();
+	  while (!mouse_thread_stdin_out)
+	    ;
+
+	  using namespace std::chrono;
+
+	  steady_clock::time_point before = steady_clock::now ();
+
+	  /* Read one cooked key out of the mouse window.  */
+	  ch = gdb_wgetch (mw);
+
+	  steady_clock::time_point after = steady_clock::now ();
+
+	  auto diff = after - before;
+	  seconds s = duration_cast<seconds> (diff);
+	  microseconds us = duration_cast<microseconds> (diff - s);
+	  if (debug_mouse_thread)
+	    fprintf (stderr, "wgetch took: %ld.%06ld\n",
+		     (long) s.count (),
+		     (long) us.count ());
+
+	  /* Tell the thread to stop reading stdin, and wait until it
+	     acknowledges it.  */
+	  mouse_thread_stdin = false;
+	  wake_mouse_thread ();
+	  while (mouse_thread_stdin_out)
+	    ;
+
+	  if (ch == ERR)
+	    {
+	      /* We get here if the previous key returned was a mouse
+		 key, and thus disabled blocked in order to flush all
+		 keys.  ERR means there are no more keys in the curses
+		 buffer.  */
+	      if (debug_mouse_thread)
+		fprintf (stderr, "got ERR\r\n");
+
+	      /* Stop draining curses.  */
+	      last_key_was_mouse = false;
+
+	      /* If the thread saw a non-mouse escape sequence, we
+		 need to drain it next.  */
+	      if (!mouse_chars.empty ())
+		{
+		  drain_mouse_chars = true;
+		  call_stdin_event_handler_again_p = 1;
+		}
+
+	      ch = 0;
+	    }
+	  else if (ch == KEY_MOUSE)
+	    {
+	      if (debug_mouse_thread)
+		fprintf (stderr, "KEY_MOUSE\n");
+
+	      /* Process this mouse key, and set up to drain other
+		 keys that ncurses may have read into its internal
+		 buffer already.  */
+	      last_key_was_mouse = true;
+	      call_stdin_event_handler_again_p = 1;
+
+	      /* Get the mouse event.  This must be called with the
+		 mouse screen as current.  */
+	      MEVENT mev;
+	      if (getmouse (&mev) == OK)
+		{
+		  /* Now handle the mouse event.  This must be done
+		     with the normal screen as current.  */
+		  set_term (prev_s);
+		  stdscr = prev_stdscr;
+
+		  /* Handle prev/next/up/down here.  */
+		  tui_dispatch_mouse_event (mev);
+		}
+
+	      return 0;
+	    }
+	  else
+	    {
+	      if (debug_mouse_thread)
+		fprintf (stderr, "not KEY_MOUSE, %c (%d)\n", ch, ch);
+
+	      last_key_was_mouse = false;
+
+	      /* Drain data buffered on the ncurses side and/or in the
+		 pipe.  */
+	      nodelay (mw, TRUE);
+
+	      /* Data already in MOUSE_CHARS must be returned to
+		 readline _after_ the data ncurses already fetched
+		 from the pipe into its own internal buffer.  IOW, the
+		 ncurses buffered data must be prepended into
+		 MOUSE_CHARS.  Start by draining ncurses data into a
+		 temporary vector.  */
+	      std::vector<int> tmp_chars;
+	      int ch2 = gdb_wgetch (mw);
+	      if (ch2 != ERR)
+		{
+		  if (debug_mouse_thread)
+		    fprintf (stderr, "more data!: %c (%d)", ch2, ch2);
+		  gdb_assert (ch2 != KEY_MOUSE);
+		  tmp_chars.push_back (ch2);
+		  while (1)
+		    {
+		      ch2 = gdb_wgetch (mw);
+		      if (ch2 == ERR)
+			{
+			  if (debug_mouse_thread)
+			    fprintf (stderr, ", ERR");
+			  break;
+			}
+		      else
+			{
+			  if (debug_mouse_thread)
+			    fprintf (stderr, ", %c (%d)", ch2, ch2);
+			  gdb_assert (ch2 != KEY_MOUSE);
+			  tmp_chars.push_back (ch2);
+			}
+		    }
+		  if (debug_mouse_thread)
+		    {
+		      fprintf (stderr, "\n");
+		      fflush (stderr);
+		    }
+		}
+
+	      nodelay (mw, FALSE);
+
+	      /* Now append the data that was already in MOUSE_CHARS
+		 in the temporary vector.  */
+	      tmp_chars.insert (tmp_chars.end (),
+				mouse_chars.begin (), mouse_chars.end ());
+
+	      /* And make the result the new MOUSE_CHARS.  */
+	      mouse_chars = std::move (tmp_chars);
+
+	      if (debug_mouse_thread)
+		fprintf (stderr, "got %c (%d), ", ch, ch);
+	      if (!mouse_chars.empty ())
+		{
+		  drain_mouse_chars = true;
+		  call_stdin_event_handler_again_p = 1;
+		}
+	    }
+
+	  /* Restore the regular screen as current.  */
+	  set_term (prev_s);
+	  stdscr = prev_stdscr;
+	}
+      else
+	{
+	  if (debug_mouse_thread)
+	    fprintf (stderr, "tui_get_c_1: not escape: %c (%d)\n", ch, ch);
+	}
+    }
+  else
+    {
+      using namespace std::chrono;
+
+      steady_clock::time_point before = steady_clock::now ();
+
+      ch = gdb_wgetch (w);
+
+      steady_clock::time_point after = steady_clock::now ();
+
+      auto diff = after - before;
+      seconds s = duration_cast<seconds> (diff);
+      microseconds us = duration_cast<microseconds> (diff - s);
+#if 0
+      fprintf (stderr, "wgetch took: %ld.%06ld\n",
+	       (long) s.count (),
+	       (long) us.count ());
+#endif
+
+      /* Handle prev/next/up/down here.  */
+      ch = tui_dispatch_ctrl_char (ch);
+    }
 
-  /* Handle prev/next/up/down here.  */
-  ch = tui_dispatch_ctrl_char (ch);
-  
   if (ch == KEY_BACKSPACE)
     return '\b';
 
+#if 0
+  /* XXX: see about re-enabling this.  */
   if (current_ui->command_editing && key_is_start_sequence (ch))
     {
       int ch_pending;
@@ -1117,6 +1607,7 @@ tui_getc_1 (FILE *fp)
 	  call_stdin_event_handler_again_p = 1;
 	}
     }
+#endif
 
   return ch;
 }
diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h
index 760532140f1..e4e9bc745b9 100644
--- a/gdb/tui/tui-io.h
+++ b/gdb/tui/tui-io.h
@@ -60,4 +60,7 @@ extern cli_ui_out *tui_old_uiout;
    next line.  */
 extern void tui_inject_newline_into_command_window ();
 
+/* Initialize the mouse screen.  */
+extern void init_mouse_screen ();
+
 #endif /* TUI_TUI_IO_H */
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 529fc62c9ac..490cdb69dda 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -383,6 +383,10 @@ tui_enable (void)
       if (!gdb_stderr->isatty ())
 	error (_("Cannot enable the TUI when output is not a terminal"));
 
+      /* This must be done before the newterm below, otherwise we end
+	 up with a messed up (main) terminal.  */
+      init_mouse_screen ();
+
       s = newterm (NULL, stdout, stdin);
 #ifdef __MINGW32__
       /* The MinGW port of ncurses requires $TERM to be unset in order

base-commit: a53755664f5f904aefd0d0b87e12f9adb6b69129
-- 
2.26.2

Re: POC: Make the TUI command window support the mouse (Re: [PATCHv3 1/2] Initial TUI mouse support)

[Hannes Domani]

 Am Samstag, 12. Juni 2021, 04:41:12 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

> On 2021-06-11 12:02 p.m., Hannes Domani wrote:
> >  Am Donnerstag, 10. Juni 2021, 20:47:01 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> >
> >>>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
> >>
> >> Hannes> I imagine the same is true for mouse escape sequences.
> >> Hannes> So I would just disable the mouse when the command window has focus, but
> >> Hannes> what do you think?
> >>
> >> I think this is a good fallback to have, but it would be better if we
> >> could make it really work somehow.  The problem with doing this is that
> >> if the command window has focus, then things like clicking in the source
> >> to set a breakpoint will not work.
> >
> > That would probably mean that keypad also has to be enabled when the
> > command window has focus.
> > I did a short test where I tried this, but then gdb crashed somewhere in
> > readline, and I stopped there, because I didn't want to debug readline code.
>
> We disable the keypad in the command window because we want to pass all escape
> sequences to readline, unprocessed by ncurses, so that e.g., up/down navigates
> the command history, left/right moves the cursor position, etc., just like if
> you weren't in the TUI.
>
> I first thought of fixing this by enabling the keypad, and then where we handle
> KEY_UP, etc., special case the command window, and call readline functions that do
> what you'd expect.  But the thing is that ends up being a poor readline emulation,
> and I didn't like where this was going.  Especially when I realized that for example,
> when you're in the middle for a reverse-search (c-r), KEY_UP does something different.
> Etc.
>
> I had a different idea to make this all work.  Warning, this is either a brilliant hack,
> or a really nasty hack, depending on perspective.  :-)
>
> The idea is to keep the keypad disabled in the command window.  However,
> we need to enable the keypad for ncurses to process mouse escape sequences.
> This is a conflict.  So here's the main trick behind the idea.  It breaks the conflict.
> Create a separate ncurses screen/terminal, with newterm, which reads from a pipe instead
> of from stdin.  Then, flush data from stdin into this pipe, BUT, make sure to never flush
> a non-mouse escape sequence into the pipe.  It's like the pipe is stdin, but with any
> non-mouse escape sequence filtered out.  This way, ncurses only either sees mouse
> escape sequences or normal ASCII keys.  In order to do this, we need to use
> a separate thread to flush stdin to the pipe.
>
> I gave this a try the other weekend, and ran into issues.  Today/tonight I gave it
> another go, and got it working.  It is a bit rough around the edges, but
> seems to work nicely.

If this works on Linux, then that's great, but Windows doesn't send mouse
escape sequences.

On the other hand, if keypad was enabled, couldn't we just forward readline
the escape sequences for the arrow keys instead?


Hannes

Re: POC: Make the TUI command window support the mouse (Re: [PATCHv3 1/2] Initial TUI mouse support)

[Pedro Alves]

On 2021-06-12 1:32 p.m., Hannes Domani wrote:

> If this works on Linux, then that's great, but Windows doesn't send mouse
> escape sequences.

I was under the impression that Windows just worked without further changes, so we'd
just not compile in the new code.

> 
> On the other hand, if keypad was enabled, couldn't we just forward readline
> the escape sequences for the arrow keys instead?

Yeah, to be honest I think that is likely a better approach and worth it of a
try -- my only concern is whether the escape sequences are standard enough
across terminals?  Maybe it's all covered by ANSI and it's all we need to care
about?  I thought of the other approach because that let's us not care about
specific sequences, other than the mouse sequence, which seemed pretty much
standard from looking around.  Also, it was run to write.  :-)

[PATCH v2] Make the TUI command window support the mouse

[Pedro Alves]

On 2021-06-12 7:08 p.m., Pedro Alves wrote:
> On 2021-06-12 1:32 p.m., Hannes Domani wrote:

>> On the other hand, if keypad was enabled, couldn't we just forward readline
>> the escape sequences for the arrow keys instead?
> 
> Yeah, to be honest I think that is likely a better approach and worth it of a
> try -- my only concern is whether the escape sequences are standard enough
> across terminals?  Maybe it's all covered by ANSI and it's all we need to care
> about?  I thought of the other approach because that let's us not care about
> specific sequences, other than the mouse sequence, which seemed pretty much
> standard from looking around.  Also, it was run to write.  :-)
> 

Alright, I gave that approach a go.  Below's a patch implementing that.  It
works quite nicely here.  I tried it on konsole, xterm and rxvt.  Among those,
rxvt uses different escape sequences for ctrl-up/ctrl-down/ctrl-left/ctrl-right,
but it doesn't really matter -- I found that readline binds actions to
different variants of escape sequences, so if we pick sequences readline always binds,
it should always work.  See readline.c:bind_arrow_keys_internal.
Despite that, for the standard ncurses keys below KEY_MAX, it's easier
to use the corresponding lowercase key_foo variable, which I believe is
filled in from termcap so should also always work, as readline also
binds the key sequences termcap returns.

Overall, I'm pleased with this approach.  See commit log for more, particularly
the extra improvement we get.

From: Pedro Alves <pedro@palves.net>
Date: 2021-06-05 19:11:09 +0100

Make the TUI command window support the mouse

Currently, when the focus is on the command window, we disable the
keypad.  This means that when the command window has the focus, keys
such as up/down/home/end etc. are not processed by ncurses, and their
escape sequences go straight to readline.

A side effect of disabling keypad mode is that wgetch no longer
processes mouse escape sequences, with the end result being the mouse
doesn't work, and worse, the raw mouse escape sequences are printed on
the terminal.

This commit makes the TUI command window support the mouse as well, by
always enabling the keypad, and then to avoid losing support for
up/down browsing the command history, home/end/left/right moving the
cursor position, etc., we forward those keys as raw escape sequences
to readline.

Note that the patch makes it so that CTLC-L is already passed to
readline even if the command window does not have the focus.  It was
simpler to implement that way, and it just seems correct to me.  I
don't know of a reason we shouldn't do that.

The patch improves the TUI behavior in a related way.  Now we can pass
keys to readline irrespective of which window has focus.  First, we
try to dispatch the key to a window, via tui_displatch_ctrl_char.  If
the key is dispatched, then we don't pass it to readline.  E.g.,
pressing "up" when you have the source window in focus results in
scrolling the source window, and nothing else.  If however, you press
ctrl-del instead, that results in killing the next word in the command
window, no matter which window has focus.  Before, it would only work
if you had the command window in focus.  Similarly,
ctrl-left/ctrl-right to move between words, etc.

Similarly, the previous spot where we handled mouse events was
incorrect.  It was never reached if the window with focus can't
scroll, which is the case for the command window.  Mouse scrolling
affects the window under the mouse cursor, not the window in focus.
We now always try to dispatch mouse events.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* tui/tui-io.c (tui_dispatch_mouse_event): New, factored out from
	...
	(tui_dispatch_ctrl_char): ... this.  Move CTRL-L handling to
	tui_getc_1.
	(cur_seq, start_sequence): New.
	(tui_getc_1): Pass key escape sequences for ncurses control keys
	to readline.  Handle mouse and ctrl-l here.
	(tui_resize_all): Disable/reenable the keypad if the command
	window has the focus too.
	* tui/tui-win.c (tui_set_focus_command): Don't change keypad
	setting.
	* tui/tui.c (tui_rl_other_window): Don't change keypad setting.

Change-Id: Ie0a7d849943cfb47f4a6589e1c73341563740fa9

---

 gdb/tui/tui-io.c  |  195 ++++++++++++++++++++++++++++++++++++++++++-----------
 gdb/tui/tui-win.c |   12 +--
 gdb/tui/tui.c     |    5 -
 3 files changed, 160 insertions(+), 52 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 7df0e2f1bd3..4da6f2bab11 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -946,6 +946,38 @@ tui_initialize_io (void)
 #endif
 }
 
+/* Dispatch the correct tui function based upon the mouse event.  */
+
+static void
+tui_dispatch_mouse_event ()
+{
+  MEVENT mev;
+  if (getmouse (&mev) != OK)
+    return;
+
+  for (tui_win_info *wi : all_tui_windows ())
+    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
+	&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
+      {
+	if ((mev.bstate & BUTTON1_CLICKED) != 0
+	    || (mev.bstate & BUTTON2_CLICKED) != 0
+	    || (mev.bstate & BUTTON3_CLICKED) != 0)
+	  {
+	    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
+	      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
+			 : 3);
+	    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
+	  }
+#ifdef BUTTON5_PRESSED
+	else if ((mev.bstate & BUTTON4_PRESSED) != 0)
+	  wi->backward_scroll (3);
+	else if ((mev.bstate & BUTTON5_PRESSED) != 0)
+	  wi->forward_scroll (3);
+#endif
+	break;
+      }
+}
+
 /* Dispatch the correct tui function based upon the control
    character.  */
 static unsigned int
@@ -953,10 +985,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
 {
   struct tui_win_info *win_info = tui_win_with_focus ();
 
-  /* Handle the CTRL-L refresh for each window.  */
-  if (ch == '\f')
-    tui_refresh_all_win ();
-
   /* If no window has the focus, or if the focus window can't scroll,
      just pass the character through.  */
   if (win_info == NULL || !win_info->can_scroll ())
@@ -984,39 +1012,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
     case KEY_LEFT:
       win_info->right_scroll (1);
       break;
-#ifdef NCURSES_MOUSE_VERSION
-    case KEY_MOUSE:
-	{
-	  MEVENT mev;
-	  if (getmouse (&mev) != OK)
-	    break;
-
-	  for (tui_win_info *wi : all_tui_windows ())
-	    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
-		&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
-	      {
-		if ((mev.bstate & BUTTON1_CLICKED) != 0
-		    || (mev.bstate & BUTTON2_CLICKED) != 0
-		    || (mev.bstate & BUTTON3_CLICKED) != 0)
-		  {
-		    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
-		      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
-				 : 3);
-		    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
-		  }
-#ifdef BUTTON5_PRESSED
-		else if ((mev.bstate & BUTTON4_PRESSED) != 0)
-		  wi->backward_scroll (3);
-		else if ((mev.bstate & BUTTON5_PRESSED) != 0)
-		  wi->forward_scroll (3);
-#endif
-		break;
-	      }
-	}
-      break;
-#endif
-    case '\f':
-      break;
     default:
       /* We didn't recognize the character as a control character, so pass it
 	 through.  */
@@ -1067,6 +1062,24 @@ tui_inject_newline_into_command_window ()
     }
 }
 
+/* If we're passing an escape sequence to readline, this points to a
+   string holding the remaining characters of the sequence to pass.
+   We advance the pointer one character at a time until '\0' is
+   reached.  */
+static const char *cur_seq = nullptr;
+
+/* Set CUR_SEQ to point at the current sequence to pass to readline,
+   setup to call the input handler again so we complete the sequence
+   shortly, and return the first character to start the sequence.  */
+
+static int
+start_sequence (const char *seq)
+{
+  call_stdin_event_handler_again_p = 1;
+  cur_seq = seq + 1;
+  return seq[0];
+}
+
 /* Main worker for tui_getc.  Get a character from the command window.
    This is called from the readline package, but wrapped in a
    try/catch by tui_getc.  */
@@ -1084,11 +1097,115 @@ tui_getc_1 (FILE *fp)
   tui_readline_output (0, 0);
 #endif
 
-  ch = gdb_wgetch (w);
+  /* We enable keypad mode so that ncurses's wgetch processes mouse
+     escape sequences.  In keypad mode, wgetch also processes the
+     escape sequences for keys such as up/down etc. and returns KEY_UP
+     / KEY_DOWN etc.  When we have the focus on the command window
+     though, we want to pass the raw up/down etc. escape codes to
+     readline so readline understands them.  */
+  if (cur_seq != nullptr)
+    {
+      ch = *cur_seq++;
+
+      /* If we've reached the end of the string, we're done with the
+	 sequence.  Otherwise, setup to get back here again for
+	 another character.  */
+      if (*cur_seq == '\0')
+	cur_seq = nullptr;
+      else
+	call_stdin_event_handler_again_p = 1;
+      return ch;
+    }
+  else
+    ch = gdb_wgetch (w);
 
   /* Handle prev/next/up/down here.  */
   ch = tui_dispatch_ctrl_char (ch);
-  
+
+#ifdef NCURSES_MOUSE_VERSION
+  if (ch == KEY_MOUSE)
+    {
+      tui_dispatch_mouse_event ();
+      return 0;
+    }
+#endif
+
+  /* Translate ncurses keys back to escape sequences so that readline
+     can understand them.  We do this irrespective of what is the
+     focus window.  If e.g., we're focused on a non-command window,
+     then the up/down keys will already have been filtered by
+     tui_dispatch_ctrl_char.  Keys that haven't been intercepted will
+     be passed down to readline.  */
+  if (current_ui->command_editing)
+    {
+      /* For the standard keys, we can find them efficiently in the
+	 key_xxx macros, defined by ncurses.  We could also hardcode
+	 sequences readline understands, and/or use rl_get_termcap.
+	 See readline/readline.c:bind_arrow_keys_internal for
+	 hardcoded sequences.  */
+      switch (ch)
+	{
+	case KEY_NPAGE: /* page down */
+	  return start_sequence (key_npage);
+	case KEY_PPAGE: /* page up */
+	  return start_sequence (key_ppage);
+	case KEY_DOWN:
+	  return start_sequence (key_down);
+	case KEY_UP:
+	  return start_sequence (key_up);
+	case KEY_RIGHT:
+	  return start_sequence (key_right);
+	case KEY_LEFT:
+	  return start_sequence (key_left);
+	case KEY_HOME:
+	  return start_sequence (key_home);
+	case KEY_END:
+	  return start_sequence (key_end);
+	case KEY_DC: /* del */
+	  return start_sequence (key_dc);
+	case KEY_IC: /* ins */
+	  return start_sequence (key_ic);
+	}
+
+      /* Keycodes above KEY_MAX are not garanteed to be stable.
+	 Compare keyname instead.  */
+      if (ch >= KEY_MAX)
+	{
+	  auto name = gdb::string_view (keyname (ch));
+
+	  /* ctrl-arrow keys */
+	  if (name == "kLFT5") /* ctrl-left */
+	    return start_sequence ("\033[1;5D");
+	  else if (name == "kRIT5") /* ctrl-right */
+	    return start_sequence ("\033[1;5C");
+	  else if (name == "kUP5") /* ctrl-up */
+	    return start_sequence ("\033[1;5A");
+	  else if (name == "kDN5") /* ctrl-down */
+	    return start_sequence ("\033[1;5B");
+	  else if (name == "kHOM5") /* ctrl-home */
+	    return start_sequence ("\033[1;5H");
+	  else if (name == "kEND5") /* ctrl-end */
+	    return start_sequence ("\033[1;5F");
+	  else if (name == "kIC5") /* ctrl-ins */
+	    return start_sequence ("\033[2;5~");
+	  else if (name == "kDC5") /* ctrl-del */
+	    return start_sequence ("\033[3;5~");
+
+	  /* alt-arrow keys */
+	  else if (name == "kLFT3") /* alt-left */
+	    return start_sequence ("\033[1;3D");
+	  else if (name == "kRIT3") /* alt-right */
+	    return start_sequence ("\033[1;3C");
+	}
+    }
+
+  /* Handle the CTRL-L refresh for each window.  */
+  if (ch == '\f')
+    {
+      tui_refresh_all_win ();
+      return ch;
+    }
+
   if (ch == KEY_BACKSPACE)
     return '\b';
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 4e75a66a00e..a51e7b9f6da 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -498,14 +498,11 @@ tui_resize_all (void)
   height_diff = screenheight - tui_term_height ();
   if (height_diff || width_diff)
     {
-      struct tui_win_info *win_with_focus = tui_win_with_focus ();
-
 #ifdef HAVE_RESIZE_TERM
       resize_term (screenheight, screenwidth);
 #endif      
       /* Turn keypad off while we resize.  */
-      if (win_with_focus != TUI_CMD_WIN)
-	keypad (TUI_CMD_WIN->handle.get (), FALSE);
+      keypad (TUI_CMD_WIN->handle.get (), FALSE);
       tui_update_gdb_sizes ();
       tui_set_term_height_to (screenheight);
       tui_set_term_width_to (screenwidth);
@@ -515,10 +512,8 @@ tui_resize_all (void)
       erase ();
       clearok (curscr, TRUE);
       tui_apply_current_layout ();
-      /* Turn keypad back on, unless focus is in the command
-	 window.  */
-      if (win_with_focus != TUI_CMD_WIN)
-	keypad (TUI_CMD_WIN->handle.get (), TRUE);
+      /* Turn keypad back on.  */
+      keypad (TUI_CMD_WIN->handle.get (), TRUE);
     }
 }
 
@@ -703,7 +698,6 @@ tui_set_focus_command (const char *arg, int from_tty)
     error (_("Window \"%s\" is not visible"), arg);
 
   tui_set_win_focus_to (win_info);
-  keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
   printf_filtered (_("Focus set to %s window.\n"),
 		   tui_win_with_focus ()->name ());
 }
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 529fc62c9ac..5f0c87c05e1 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -179,10 +179,7 @@ tui_rl_other_window (int count, int key)
 
   win_info = tui_next_win (tui_win_with_focus ());
   if (win_info)
-    {
-      tui_set_win_focus_to (win_info);
-      keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
-    }
+    tui_set_win_focus_to (win_info);
   return 0;
 }
 

Re: [PATCH v2] Make the TUI command window support the mouse

[Eli Zaretskii]

> From: Pedro Alves <pedro@palves.net>
> Date: Sun, 13 Jun 2021 03:46:13 +0100
> Cc: Joel Brobecker <brobecker@adacore.com>
> 
> On 2021-06-12 7:08 p.m., Pedro Alves wrote:
> > On 2021-06-12 1:32 p.m., Hannes Domani wrote:
> 
> >> On the other hand, if keypad was enabled, couldn't we just forward readline
> >> the escape sequences for the arrow keys instead?
> > 
> > Yeah, to be honest I think that is likely a better approach and worth it of a
> > try -- my only concern is whether the escape sequences are standard enough
> > across terminals?  Maybe it's all covered by ANSI and it's all we need to care
> > about?  I thought of the other approach because that let's us not care about
> > specific sequences, other than the mouse sequence, which seemed pretty much
> > standard from looking around.  Also, it was run to write.  :-)
> > 
> 
> Alright, I gave that approach a go.  Below's a patch implementing that.  It
> works quite nicely here.  I tried it on konsole, xterm and rxvt.  Among those,
> rxvt uses different escape sequences for ctrl-up/ctrl-down/ctrl-left/ctrl-right,
> but it doesn't really matter -- I found that readline binds actions to
> different variants of escape sequences, so if we pick sequences readline always binds,
> it should always work.  See readline.c:bind_arrow_keys_internal.
> Despite that, for the standard ncurses keys below KEY_MAX, it's easier
> to use the corresponding lowercase key_foo variable, which I believe is
> filled in from termcap so should also always work, as readline also
> binds the key sequences termcap returns.

Maybe I'm missing something, but what about MS-Windows, where the
cursor motion keys don't (AFAIK) generate escape sequences?

Re: [PATCH v2] Make the TUI command window support the mouse

[Pedro Alves]

On 2021-06-13 11:35 a.m., Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Date: Sun, 13 Jun 2021 03:46:13 +0100
>> Cc: Joel Brobecker <brobecker@adacore.com>
>>
>> On 2021-06-12 7:08 p.m., Pedro Alves wrote:
>>> On 2021-06-12 1:32 p.m., Hannes Domani wrote:
>>
>>>> On the other hand, if keypad was enabled, couldn't we just forward readline
>>>> the escape sequences for the arrow keys instead?
>>>
>>> Yeah, to be honest I think that is likely a better approach and worth it of a
>>> try -- my only concern is whether the escape sequences are standard enough
>>> across terminals?  Maybe it's all covered by ANSI and it's all we need to care
>>> about?  I thought of the other approach because that let's us not care about
>>> specific sequences, other than the mouse sequence, which seemed pretty much
>>> standard from looking around.  Also, it was run to write.  :-)
>>>
>>
>> Alright, I gave that approach a go.  Below's a patch implementing that.  It
>> works quite nicely here.  I tried it on konsole, xterm and rxvt.  Among those,
>> rxvt uses different escape sequences for ctrl-up/ctrl-down/ctrl-left/ctrl-right,
>> but it doesn't really matter -- I found that readline binds actions to
>> different variants of escape sequences, so if we pick sequences readline always binds,
>> it should always work.  See readline.c:bind_arrow_keys_internal.
>> Despite that, for the standard ncurses keys below KEY_MAX, it's easier
>> to use the corresponding lowercase key_foo variable, which I believe is
>> filled in from termcap so should also always work, as readline also
>> binds the key sequences termcap returns.
> 
> Maybe I'm missing something, but what about MS-Windows, where the
> cursor motion keys don't (AFAIK) generate escape sequences?

AFAICT, readline processes the escape sequences we're passing it anyhow,
since it unconditionally registers/binds them.  It seems to be working for
Hannes.

Pedro Alves

Re: [PATCH v2] Make the TUI command window support the mouse

[Eli Zaretskii]

> From: Pedro Alves <pedro@palves.net>
> Cc: ssbssa@yahoo.de, gdb-patches@sourceware.org, tom@tromey.com,
>  brobecker@adacore.com
> Date: Sun, 13 Jun 2021 18:29:57 +0100
> 
> > Maybe I'm missing something, but what about MS-Windows, where the
> > cursor motion keys don't (AFAIK) generate escape sequences?
> 
> AFAICT, readline processes the escape sequences we're passing it anyhow,
> since it unconditionally registers/binds them.  It seems to be working for
> Hannes.

AFAIR, Hannes uses PDCurses, not ncurses, and I don't know what that
means for this particular issue.  I didn't try building GDB after
these changes, but if someone succeeded building with ncurses and
running this code on Windows, then I'm happy, of course.

Re: [PATCH v2] Make the TUI command window support the mouse

[Pedro Alves]

On 2021-06-13 7:02 p.m., Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Cc: ssbssa@yahoo.de, gdb-patches@sourceware.org, tom@tromey.com,
>>  brobecker@adacore.com
>> Date: Sun, 13 Jun 2021 18:29:57 +0100
>>
>>> Maybe I'm missing something, but what about MS-Windows, where the
>>> cursor motion keys don't (AFAIK) generate escape sequences?
>>
>> AFAICT, readline processes the escape sequences we're passing it anyhow,
>> since it unconditionally registers/binds them.  It seems to be working for
>> Hannes.
> 
> AFAIR, Hannes uses PDCurses, not ncurses, and I don't know what that
> means for this particular issue.  I didn't try building GDB after
> these changes, but if someone succeeded building with ncurses and
> running this code on Windows, then I'm happy, of course.
> 

Note we're feeding readline the escape sequences when it reads
input (which ends up in a callback in the tui to return the next character
out of stdin), not expecting to see those escape sequences directly out of stdin.
I don't think the curses library readline is linked with will make a difference, since
it's readline that processes the escape codes we feed it, not the curses library.  readline
uses the curses library as a way to access termcap (if linked that way), in order to
bind _other_ escape sequences.  The sequences we're passing to readline are always bound
by readline without consulting termcap, they're hardcoded as in "readline always recognizes
these escape sequences".  It won't hurt to test it, of course.

Pedro Alves

Re: [PATCH v2] Make the TUI command window support the mouse

[Hannes Domani]

 Am Sonntag, 13. Juni 2021, 04:46:15 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

> On 2021-06-12 7:08 p.m., Pedro Alves wrote:
> > On 2021-06-12 1:32 p.m., Hannes Domani wrote:
>
> >> On the other hand, if keypad was enabled, couldn't we just forward readline
> >> the escape sequences for the arrow keys instead?
> >
> > Yeah, to be honest I think that is likely a better approach and worth it of a
> > try -- my only concern is whether the escape sequences are standard enough
> > across terminals?  Maybe it's all covered by ANSI and it's all we need to care
> > about?  I thought of the other approach because that let's us not care about
> > specific sequences, other than the mouse sequence, which seemed pretty much
> > standard from looking around.  Also, it was run to write.  :-)
> >
>
> Alright, I gave that approach a go.  Below's a patch implementing that.  It
> works quite nicely here.  I tried it on konsole, xterm and rxvt.  Among those,
> rxvt uses different escape sequences for ctrl-up/ctrl-down/ctrl-left/ctrl-right,
> but it doesn't really matter -- I found that readline binds actions to
> different variants of escape sequences, so if we pick sequences readline always binds,
> it should always work.  See readline.c:bind_arrow_keys_internal.
> Despite that, for the standard ncurses keys below KEY_MAX, it's easier
> to use the corresponding lowercase key_foo variable, which I believe is
> filled in from termcap so should also always work, as readline also
> binds the key sequences termcap returns.
>
> Overall, I'm pleased with this approach.  See commit log for more, particularly
> the extra improvement we get.
>
> From: Pedro Alves <pedro@palves.net>
> Date: 2021-06-05 19:11:09 +0100
>
> Make the TUI command window support the mouse
>
> Currently, when the focus is on the command window, we disable the
> keypad.  This means that when the command window has the focus, keys
> such as up/down/home/end etc. are not processed by ncurses, and their
> escape sequences go straight to readline.
>
> A side effect of disabling keypad mode is that wgetch no longer
> processes mouse escape sequences, with the end result being the mouse
> doesn't work, and worse, the raw mouse escape sequences are printed on
> the terminal.
>
> This commit makes the TUI command window support the mouse as well, by
> always enabling the keypad, and then to avoid losing support for
> up/down browsing the command history, home/end/left/right moving the
> cursor position, etc., we forward those keys as raw escape sequences
> to readline.
>
> Note that the patch makes it so that CTLC-L is already passed to
> readline even if the command window does not have the focus.  It was
> simpler to implement that way, and it just seems correct to me.  I
> don't know of a reason we shouldn't do that.
>
> The patch improves the TUI behavior in a related way.  Now we can pass
> keys to readline irrespective of which window has focus.  First, we
> try to dispatch the key to a window, via tui_displatch_ctrl_char.  If
> the key is dispatched, then we don't pass it to readline.  E.g.,
> pressing "up" when you have the source window in focus results in
> scrolling the source window, and nothing else.  If however, you press
> ctrl-del instead, that results in killing the next word in the command
> window, no matter which window has focus.  Before, it would only work
> if you had the command window in focus.  Similarly,
> ctrl-left/ctrl-right to move between words, etc.
>
> Similarly, the previous spot where we handled mouse events was
> incorrect.  It was never reached if the window with focus can't
> scroll, which is the case for the command window.  Mouse scrolling
> affects the window under the mouse cursor, not the window in focus.
> We now always try to dispatch mouse events.

This is great, thank you for doing this.

I just tried it, and I had just one problem, see below.


> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <pedro@palves.net>
>
>     * tui/tui-io.c (tui_dispatch_mouse_event): New, factored out from
>     ...
>     (tui_dispatch_ctrl_char): ... this.  Move CTRL-L handling to
>     tui_getc_1.
>     (cur_seq, start_sequence): New.
>     (tui_getc_1): Pass key escape sequences for ncurses control keys
>     to readline.  Handle mouse and ctrl-l here.
>     (tui_resize_all): Disable/reenable the keypad if the command
>     window has the focus too.
>     * tui/tui-win.c (tui_set_focus_command): Don't change keypad
>     setting.
>     * tui/tui.c (tui_rl_other_window): Don't change keypad setting.
>
> Change-Id: Ie0a7d849943cfb47f4a6589e1c73341563740fa9
>
> ---
>
> gdb/tui/tui-io.c  |  195 ++++++++++++++++++++++++++++++++++++++++++-----------
> gdb/tui/tui-win.c |  12 +--
> gdb/tui/tui.c    |    5 -
> 3 files changed, 160 insertions(+), 52 deletions(-)
>
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index 7df0e2f1bd3..4da6f2bab11 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -946,6 +946,38 @@ tui_initialize_io (void)
> #endif
> }
>
> +/* Dispatch the correct tui function based upon the mouse event.  */
> +
> +static void
> +tui_dispatch_mouse_event ()
> +{
> +  MEVENT mev;
> +  if (getmouse (&mev) != OK)
> +    return;
> +
> +  for (tui_win_info *wi : all_tui_windows ())
> +    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
> +    && mev.y > wi->y && mev.y < wi->y + wi->height - 1)
> +      {
> +    if ((mev.bstate & BUTTON1_CLICKED) != 0
> +        || (mev.bstate & BUTTON2_CLICKED) != 0
> +        || (mev.bstate & BUTTON3_CLICKED) != 0)
> +      {
> +        int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
> +          :        ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
> +            : 3);
> +        wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
> +      }
> +#ifdef BUTTON5_PRESSED
> +    else if ((mev.bstate & BUTTON4_PRESSED) != 0)
> +      wi->backward_scroll (3);
> +    else if ((mev.bstate & BUTTON5_PRESSED) != 0)
> +      wi->forward_scroll (3);
> +#endif
> +    break;
> +      }
> +}
> +
> /* Dispatch the correct tui function based upon the control
>     character.  */
> static unsigned int
> @@ -953,10 +985,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
> {
>   struct tui_win_info *win_info = tui_win_with_focus ();
>
> -  /* Handle the CTRL-L refresh for each window.  */
> -  if (ch == '\f')
> -    tui_refresh_all_win ();
> -
>   /* If no window has the focus, or if the focus window can't scroll,
>       just pass the character through.  */
>   if (win_info == NULL || !win_info->can_scroll ())
> @@ -984,39 +1012,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
>     case KEY_LEFT:
>       win_info->right_scroll (1);
>       break;
> -#ifdef NCURSES_MOUSE_VERSION
> -    case KEY_MOUSE:
> -    {
> -      MEVENT mev;
> -      if (getmouse (&mev) != OK)
> -        break;
> -
> -      for (tui_win_info *wi : all_tui_windows ())
> -        if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
> -        && mev.y > wi->y && mev.y < wi->y + wi->height - 1)
> -          {
> -        if ((mev.bstate & BUTTON1_CLICKED) != 0
> -            || (mev.bstate & BUTTON2_CLICKED) != 0
> -            || (mev.bstate & BUTTON3_CLICKED) != 0)
> -          {
> -            int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
> -              :        ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
>
> -                : 3);
>
> -            wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
> -          }
> -#ifdef BUTTON5_PRESSED
> -        else if ((mev.bstate & BUTTON4_PRESSED) != 0)
> -          wi->backward_scroll (3);
> -        else if ((mev.bstate & BUTTON5_PRESSED) != 0)
> -          wi->forward_scroll (3);
> -#endif
> -        break;
> -          }
> -    }
> -      break;
> -#endif
> -    case '\f':
> -      break;
>     default:
>       /* We didn't recognize the character as a control character, so pass it
>     through.  */
> @@ -1067,6 +1062,24 @@ tui_inject_newline_into_command_window ()
>     }
> }
>
> +/* If we're passing an escape sequence to readline, this points to a
> +  string holding the remaining characters of the sequence to pass.
> +  We advance the pointer one character at a time until '\0' is
> +  reached.  */
> +static const char *cur_seq = nullptr;
> +
> +/* Set CUR_SEQ to point at the current sequence to pass to readline,
> +  setup to call the input handler again so we complete the sequence
> +  shortly, and return the first character to start the sequence.  */
> +
> +static int
> +start_sequence (const char *seq)
> +{
> +  call_stdin_event_handler_again_p = 1;
> +  cur_seq = seq + 1;
> +  return seq[0];
> +}
> +
> /* Main worker for tui_getc.  Get a character from the command window.
>     This is called from the readline package, but wrapped in a
>     try/catch by tui_getc.  */
> @@ -1084,11 +1097,115 @@ tui_getc_1 (FILE *fp)
>   tui_readline_output (0, 0);
> #endif
>
> -  ch = gdb_wgetch (w);
> +  /* We enable keypad mode so that ncurses's wgetch processes mouse
> +    escape sequences.  In keypad mode, wgetch also processes the
> +    escape sequences for keys such as up/down etc. and returns KEY_UP
> +    / KEY_DOWN etc.  When we have the focus on the command window
> +    though, we want to pass the raw up/down etc. escape codes to
> +    readline so readline understands them.  */
> +  if (cur_seq != nullptr)
> +    {
> +      ch = *cur_seq++;
> +
> +      /* If we've reached the end of the string, we're done with the
> +    sequence.  Otherwise, setup to get back here again for
> +    another character.  */
> +      if (*cur_seq == '\0')
> +    cur_seq = nullptr;
> +      else
> +    call_stdin_event_handler_again_p = 1;
> +      return ch;
> +    }
> +  else
> +    ch = gdb_wgetch (w);
>
>   /* Handle prev/next/up/down here.  */
>   ch = tui_dispatch_ctrl_char (ch);
> -
> +
> +#ifdef NCURSES_MOUSE_VERSION
> +  if (ch == KEY_MOUSE)
> +    {
> +      tui_dispatch_mouse_event ();
> +      return 0;
> +    }
> +#endif
> +
> +  /* Translate ncurses keys back to escape sequences so that readline
> +    can understand them.  We do this irrespective of what is the
> +    focus window.  If e.g., we're focused on a non-command window,
> +    then the up/down keys will already have been filtered by
> +    tui_dispatch_ctrl_char.  Keys that haven't been intercepted will
> +    be passed down to readline.  */
> +  if (current_ui->command_editing)
> +    {
> +      /* For the standard keys, we can find them efficiently in the
> +    key_xxx macros, defined by ncurses.  We could also hardcode
> +    sequences readline understands, and/or use rl_get_termcap.
> +    See readline/readline.c:bind_arrow_keys_internal for
> +    hardcoded sequences.  */
> +      switch (ch)
> +    {
> +    case KEY_NPAGE: /* page down */
> +      return start_sequence (key_npage);
> +    case KEY_PPAGE: /* page up */
> +      return start_sequence (key_ppage);
> +    case KEY_DOWN:
> +      return start_sequence (key_down);
> +    case KEY_UP:
> +      return start_sequence (key_up);
> +    case KEY_RIGHT:
> +      return start_sequence (key_right);
> +    case KEY_LEFT:
> +      return start_sequence (key_left);
> +    case KEY_HOME:
> +      return start_sequence (key_home);
> +    case KEY_END:
> +      return start_sequence (key_end);
> +    case KEY_DC: /* del */
> +      return start_sequence (key_dc);
> +    case KEY_IC: /* ins */
> +      return start_sequence (key_ic);
> +    }

PDcurses doesn't have these key_npage/key_ppage/key_down/... variables,
so I had to use the hardcoded sequences as you described.
This is working for the arrow keys and home/end, and for del I used '\004'
instead, but I didn't see any for page up/down and ins.

I'm not sure if it's even a problem for page up/down, they seem to not be used
by readline, but I couldn't make ins work.

And rl_get_termcap returned for everything except "le" NULL, but I assume
that's just how it is on Windows.


> +
> +      /* Keycodes above KEY_MAX are not garanteed to be stable.
> +    Compare keyname instead.  */
> +      if (ch >= KEY_MAX)
> +    {
> +      auto name = gdb::string_view (keyname (ch));
> +
> +      /* ctrl-arrow keys */
> +      if (name == "kLFT5") /* ctrl-left */
> +        return start_sequence ("\033[1;5D");
> +      else if (name == "kRIT5") /* ctrl-right */
> +        return start_sequence ("\033[1;5C");
> +      else if (name == "kUP5") /* ctrl-up */
> +        return start_sequence ("\033[1;5A");
> +      else if (name == "kDN5") /* ctrl-down */
> +        return start_sequence ("\033[1;5B");
> +      else if (name == "kHOM5") /* ctrl-home */
> +        return start_sequence ("\033[1;5H");
> +      else if (name == "kEND5") /* ctrl-end */
> +        return start_sequence ("\033[1;5F");
> +      else if (name == "kIC5") /* ctrl-ins */
> +        return start_sequence ("\033[2;5~");
> +      else if (name == "kDC5") /* ctrl-del */
> +        return start_sequence ("\033[3;5~");
> +
> +      /* alt-arrow keys */
> +      else if (name == "kLFT3") /* alt-left */
> +        return start_sequence ("\033[1;3D");
> +      else if (name == "kRIT3") /* alt-right */
> +        return start_sequence ("\033[1;3C");
> +    }
> +    }
> +
> +  /* Handle the CTRL-L refresh for each window.  */
> +  if (ch == '\f')
> +    {
> +      tui_refresh_all_win ();
> +      return ch;
> +    }
> +
>   if (ch == KEY_BACKSPACE)
>     return '\b';
>
> diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
> index 4e75a66a00e..a51e7b9f6da 100644
> --- a/gdb/tui/tui-win.c
> +++ b/gdb/tui/tui-win.c
> @@ -498,14 +498,11 @@ tui_resize_all (void)
>   height_diff = screenheight - tui_term_height ();
>   if (height_diff || width_diff)
>     {
> -      struct tui_win_info *win_with_focus = tui_win_with_focus ();
> -
> #ifdef HAVE_RESIZE_TERM
>       resize_term (screenheight, screenwidth);
> #endif     
>       /* Turn keypad off while we resize.  */
> -      if (win_with_focus != TUI_CMD_WIN)
> -    keypad (TUI_CMD_WIN->handle.get (), FALSE);
> +      keypad (TUI_CMD_WIN->handle.get (), FALSE);
>       tui_update_gdb_sizes ();
>       tui_set_term_height_to (screenheight);
>       tui_set_term_width_to (screenwidth);
> @@ -515,10 +512,8 @@ tui_resize_all (void)
>       erase ();
>       clearok (curscr, TRUE);
>       tui_apply_current_layout ();
> -      /* Turn keypad back on, unless focus is in the command
> -    window.  */
> -      if (win_with_focus != TUI_CMD_WIN)
> -    keypad (TUI_CMD_WIN->handle.get (), TRUE);
> +      /* Turn keypad back on.  */
> +      keypad (TUI_CMD_WIN->handle.get (), TRUE);
>     }
> }
>
> @@ -703,7 +698,6 @@ tui_set_focus_command (const char *arg, int from_tty)
>     error (_("Window \"%s\" is not visible"), arg);
>
>   tui_set_win_focus_to (win_info);
> -  keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
>   printf_filtered (_("Focus set to %s window.\n"),
>           tui_win_with_focus ()->name ());
> }
> diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
> index 529fc62c9ac..5f0c87c05e1 100644
> --- a/gdb/tui/tui.c
> +++ b/gdb/tui/tui.c
> @@ -179,10 +179,7 @@ tui_rl_other_window (int count, int key)
>
>   win_info = tui_next_win (tui_win_with_focus ());
>   if (win_info)
> -    {
> -      tui_set_win_focus_to (win_info);
> -      keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
> -    }
> +    tui_set_win_focus_to (win_info);
>   return 0;
>
> }


Hannes

[PATCH v3] Make the TUI command window support the mouse

[Pedro Alves]

On 2021-06-13 2:04 p.m., Hannes Domani wrote:
>  Am Sonntag, 13. Juni 2021, 04:46:15 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:
> 
> This is great, thank you for doing this.
> 
> I just tried it, and I had just one problem, see below.
> 

>> +      /* For the standard keys, we can find them efficiently in the
>> +    key_xxx macros, defined by ncurses.  We could also hardcode
>> +    sequences readline understands, and/or use rl_get_termcap.
>> +    See readline/readline.c:bind_arrow_keys_internal for
>> +    hardcoded sequences.  */
>> +      switch (ch)
>> +    {
>> +    case KEY_NPAGE: /* page down */
>> +      return start_sequence (key_npage);
>> +    case KEY_PPAGE: /* page up */
>> +      return start_sequence (key_ppage);
>> +    case KEY_DOWN:
>> +      return start_sequence (key_down);
>> +    case KEY_UP:
>> +      return start_sequence (key_up);
>> +    case KEY_RIGHT:
>> +      return start_sequence (key_right);
>> +    case KEY_LEFT:
>> +      return start_sequence (key_left);
>> +    case KEY_HOME:
>> +      return start_sequence (key_home);
>> +    case KEY_END:
>> +      return start_sequence (key_end);
>> +    case KEY_DC: /* del */
>> +      return start_sequence (key_dc);
>> +    case KEY_IC: /* ins */
>> +      return start_sequence (key_ic);
>> +    }
> 
> PDcurses doesn't have these key_npage/key_ppage/key_down/... variables,
> so I had to use the hardcoded sequences as you described.
> This is working for the arrow keys and home/end, and for del I used '\004'
> instead, but I didn't see any for page up/down and ins.

Ack.  \004 is ctrl-d  ('d' & 0x1f).   I'm a bit wary of hardcoding it,
since I imagine there might be some users out where that have bound
ctrl-d to some other action in their inputrc.

readline's bind_arrow_keys_internal has:

#if defined (__MINGW32__)
  rl_bind_keyseq_if_unbound ("\340H", rl_get_previous_history);
  rl_bind_keyseq_if_unbound ("\340P", rl_get_next_history);
  rl_bind_keyseq_if_unbound ("\340M", rl_forward_char);
  rl_bind_keyseq_if_unbound ("\340K", rl_backward_char);
  rl_bind_keyseq_if_unbound ("\340G", rl_beg_of_line);
  rl_bind_keyseq_if_unbound ("\340O", rl_end_of_line);
  rl_bind_keyseq_if_unbound ("\340S", rl_delete);
  rl_bind_keyseq_if_unbound ("\340R", rl_overwrite_mode);

  /* These may or may not work because of the embedded NUL. */
  rl_bind_keyseq_if_unbound ("\\000H", rl_get_previous_history);
  rl_bind_keyseq_if_unbound ("\\000P", rl_get_next_history);
  rl_bind_keyseq_if_unbound ("\\000M", rl_forward_char);
  rl_bind_keyseq_if_unbound ("\\000K", rl_backward_char);
  rl_bind_keyseq_if_unbound ("\\000G", rl_beg_of_line);
  rl_bind_keyseq_if_unbound ("\\000O", rl_end_of_line);
  rl_bind_keyseq_if_unbound ("\\000S", rl_delete);
  rl_bind_keyseq_if_unbound ("\\000R", rl_overwrite_mode);
#endif

So would "\340S" and "\340R" work instead?  See attached patch.

> 
> I'm not sure if it's even a problem for page up/down, they seem to not be used
> by readline, but I couldn't make ins work.

Ins works for me on konsole, and on a linux virtual console.  It does not work
on xterm and rxvt.  Nor on konsole + ssh.  Nor under screen.  I confirmed that
I get the same key sequence for the insert key on all those cases (run "cat" + press
the key).  It's the same with bash and gdb in non-tui mode.  It's not specific to this
patch.  I would guess readline is somehow finding that the terminal does not support
overwrite mode.

> 
> And rl_get_termcap returned for everything except "le" NULL, but I assume
> that's just how it is on Windows.

Unfortunate.

Here's a new version.  Does this one work for you?

From dca7d16331980842ec022ed83003fbf7cb90575e Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Sat, 5 Jun 2021 19:11:09 +0100
Subject: [PATCH v3] Make the TUI command window support the mouse

In v3:
 - hardcode sequences instead of using ncurses's key_up etc. which
   don't exist on pdcurses.
 - drop page up/ page down and ctrl combinations
 - never pass non-ascii characters to readline
 - add missing NCURSES_MOUSE_VERSION check in tui_dispatch_mouse_event.
 - misc minor comment tweaks

Currently, when the focus is on the command window, we disable the
keypad.  This means that when the command window has the focus, keys
such as up/down/home/end etc. are not processed by curses, and their
escape sequences go straight to readline.

A side effect of disabling keypad mode is that wgetch no longer
processes mouse escape sequences, with the end result being the mouse
doesn't work, and worse, the raw mouse escape sequences are printed on
the terminal.

This commit makes the TUI command window support the mouse as well, by
always enabling the keypad, and then to avoid losing support for
up/down browsing the command history, home/end/left/right moving the
cursor position, etc., we forward those keys as raw escape sequences
to readline.  Note we don't make an effort to pass down to readline
all keys returned by curses, only the common ones that readline
understands by default.  Given users can specify their own readline
bindings (inputrc file, bind utility), this approach is good in
practice, though not 100% transparent or perfect.

Note that the patch makes it so that CTLC-L is always passed to
readline even if the command window does not have the focus.  It was
simpler to implement that way, and it just seems correct to me.  I
don't know of a reason we shouldn't do that.

The patch improves the TUI behavior in a related way.  Now we can pass
special keys to readline irrespective of which window has the focus.
First, we try to dispatch the key to a window, via
tui_displatch_ctrl_char.  If the key is dispatched, then we don't pass
it to readline.  E.g., pressing "up" when you have the source window
in focus results in scrolling the source window, and nothing else.  If
however, you press ctrl-del instead, that results in killing the next
word in the command window, no matter which window has has focus.
Before, it would only work if you had the command window in focus.
Similarly, ctrl-left/ctrl-right to move between words, etc.

Similarly, the previous spot where we handled mouse events was
incorrect.  It was never reached if the window with focus can't
scroll, which is the case for the command window.  Mouse scrolling
affects the window under the mouse cursor, not the window in focus.
We now always try to dispatch mouse events.

One last bit in the patch -- now if we don't recognize the non-ascii
curses key, then we don't pass it down to readline at all.  Before
that would result in bogus characters in the input line.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* tui/tui-io.c (tui_dispatch_mouse_event): New, factored out from
	...
	(tui_dispatch_ctrl_char): ... this.  Move CTRL-L handling to
	tui_getc_1.
	(cur_seq, start_sequence): New.
	(tui_getc_1): Pass key escape sequences for curses control keys to
	readline.  Handle mouse and ctrl-l here.
	(tui_resize_all): Disable/reenable the keypad if the command
	window has the focus too.
	* tui/tui-win.c (tui_set_focus_command): Don't change keypad
	setting.
	* tui/tui.c (tui_rl_other_window): Don't change keypad setting.

Change-Id: Ie0a7d849943cfb47f4a6589e1c73341563740fa9
---
 gdb/tui/tui-io.c  | 204 +++++++++++++++++++++++++++++++++++++---------
 gdb/tui/tui-win.c |  12 +--
 gdb/tui/tui.c     |   5 +-
 3 files changed, 169 insertions(+), 52 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 7df0e2f1bd3..d16a121401c 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -946,6 +946,40 @@ tui_initialize_io (void)
 #endif
 }
 
+/* Dispatch the correct tui function based upon the mouse event.  */
+
+static void
+tui_dispatch_mouse_event ()
+{
+#ifdef NCURSES_MOUSE_VERSION
+  MEVENT mev;
+  if (getmouse (&mev) != OK)
+    return;
+
+  for (tui_win_info *wi : all_tui_windows ())
+    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
+	&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
+      {
+	if ((mev.bstate & BUTTON1_CLICKED) != 0
+	    || (mev.bstate & BUTTON2_CLICKED) != 0
+	    || (mev.bstate & BUTTON3_CLICKED) != 0)
+	  {
+	    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
+	      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
+			 : 3);
+	    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
+	  }
+#ifdef BUTTON5_PRESSED
+	else if ((mev.bstate & BUTTON4_PRESSED) != 0)
+	  wi->backward_scroll (3);
+	else if ((mev.bstate & BUTTON5_PRESSED) != 0)
+	  wi->forward_scroll (3);
+#endif
+	break;
+      }
+#endif
+}
+
 /* Dispatch the correct tui function based upon the control
    character.  */
 static unsigned int
@@ -953,10 +987,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
 {
   struct tui_win_info *win_info = tui_win_with_focus ();
 
-  /* Handle the CTRL-L refresh for each window.  */
-  if (ch == '\f')
-    tui_refresh_all_win ();
-
   /* If no window has the focus, or if the focus window can't scroll,
      just pass the character through.  */
   if (win_info == NULL || !win_info->can_scroll ())
@@ -984,39 +1014,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
     case KEY_LEFT:
       win_info->right_scroll (1);
       break;
-#ifdef NCURSES_MOUSE_VERSION
-    case KEY_MOUSE:
-	{
-	  MEVENT mev;
-	  if (getmouse (&mev) != OK)
-	    break;
-
-	  for (tui_win_info *wi : all_tui_windows ())
-	    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
-		&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
-	      {
-		if ((mev.bstate & BUTTON1_CLICKED) != 0
-		    || (mev.bstate & BUTTON2_CLICKED) != 0
-		    || (mev.bstate & BUTTON3_CLICKED) != 0)
-		  {
-		    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
-		      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
-				 : 3);
-		    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
-		  }
-#ifdef BUTTON5_PRESSED
-		else if ((mev.bstate & BUTTON4_PRESSED) != 0)
-		  wi->backward_scroll (3);
-		else if ((mev.bstate & BUTTON5_PRESSED) != 0)
-		  wi->forward_scroll (3);
-#endif
-		break;
-	      }
-	}
-      break;
-#endif
-    case '\f':
-      break;
     default:
       /* We didn't recognize the character as a control character, so pass it
 	 through.  */
@@ -1067,6 +1064,24 @@ tui_inject_newline_into_command_window ()
     }
 }
 
+/* If we're passing an escape sequence to readline, this points to a
+   string holding the remaining characters of the sequence to pass.
+   We advance the pointer one character at a time until '\0' is
+   reached.  */
+static const char *cur_seq = nullptr;
+
+/* Set CUR_SEQ to point at the current sequence to pass to readline,
+   setup to call the input handler again so we complete the sequence
+   shortly, and return the first character to start the sequence.  */
+
+static int
+start_sequence (const char *seq)
+{
+  call_stdin_event_handler_again_p = 1;
+  cur_seq = seq + 1;
+  return seq[0];
+}
+
 /* Main worker for tui_getc.  Get a character from the command window.
    This is called from the readline package, but wrapped in a
    try/catch by tui_getc.  */
@@ -1084,11 +1099,115 @@ tui_getc_1 (FILE *fp)
   tui_readline_output (0, 0);
 #endif
 
-  ch = gdb_wgetch (w);
+  /* We enable keypad mode so that curses's wgetch processes mouse
+     escape sequences.  In keypad mode, wgetch also processes the
+     escape sequences for keys such as up/down etc. and returns KEY_UP
+     / KEY_DOWN etc.  When we have the focus on the command window
+     though, we want to pass the raw up/down etc. escape codes to
+     readline so readline understands them.  */
+  if (cur_seq != nullptr)
+    {
+      ch = *cur_seq++;
+
+      /* If we've reached the end of the string, we're done with the
+	 sequence.  Otherwise, setup to get back here again for
+	 another character.  */
+      if (*cur_seq == '\0')
+	cur_seq = nullptr;
+      else
+	call_stdin_event_handler_again_p = 1;
+      return ch;
+    }
+  else
+    ch = gdb_wgetch (w);
 
   /* Handle prev/next/up/down here.  */
   ch = tui_dispatch_ctrl_char (ch);
-  
+
+#ifdef NCURSES_MOUSE_VERSION
+  if (ch == KEY_MOUSE)
+    {
+      tui_dispatch_mouse_event ();
+      return 0;
+    }
+#endif
+
+  /* Translate curses keys back to escape sequences so that readline
+     can understand them.  We do this irrespective of which window has
+     the focus.  If e.g., we're focused on a non-command window, then
+     the up/down keys will already have been filtered by
+     tui_dispatch_ctrl_char.  Keys that haven't been intercepted will
+     be passed down to readline.  */
+  if (current_ui->command_editing)
+    {
+      /* For the standard arrow keys + home/end, hardcode sequences
+	 readline understands.  See bind_arrow_keys_internal in
+	 readline/readline.c.  */
+      switch (ch)
+	{
+	case KEY_UP:
+	  return start_sequence ("\033[A");
+	case KEY_DOWN:
+	  return start_sequence ("\033[B");
+	case KEY_RIGHT:
+	  return start_sequence ("\033[C");
+	case KEY_LEFT:
+	  return start_sequence ("\033[D");
+	case KEY_HOME:
+	  return start_sequence ("\033[H");
+	case KEY_END:
+	  return start_sequence ("\033[F");
+
+	/* del and ins are unfortunately not hardcoded in readline for
+	   all systems.  */
+
+	case KEY_DC: /* del */
+#ifdef __MINGW32__
+	  return start_sequence ("\340S");
+#else
+	  return start_sequence ("\e[3~");
+#endif
+
+	case KEY_IC: /* ins */
+#if defined __MINGW32__
+	  return start_sequence ("\340R");
+#else
+	  return start_sequence ("\e[2~");
+#endif
+	}
+
+      /* Keycodes above KEY_MAX are not garanteed to be stable.
+	 Compare keyname instead.  */
+      if (ch >= KEY_MAX)
+	{
+	  auto name = gdb::string_view (keyname (ch));
+
+	  /* The following sequences are hardcoded in readline as
+	     well.  */
+
+	  /* ctrl-arrow keys */
+	  if (name == "kLFT5") /* ctrl-left */
+	    return start_sequence ("\033[1;5D");
+	  else if (name == "kRIT5") /* ctrl-right */
+	    return start_sequence ("\033[1;5C");
+	  else if (name == "kDC5") /* ctrl-del */
+	    return start_sequence ("\033[3;5~");
+
+	  /* alt-arrow keys */
+	  else if (name == "kLFT3") /* alt-left */
+	    return start_sequence ("\033[1;3D");
+	  else if (name == "kRIT3") /* alt-right */
+	    return start_sequence ("\033[1;3C");
+	}
+    }
+
+  /* Handle the CTRL-L refresh for each window.  */
+  if (ch == '\f')
+    {
+      tui_refresh_all_win ();
+      return ch;
+    }
+
   if (ch == KEY_BACKSPACE)
     return '\b';
 
@@ -1118,6 +1237,13 @@ tui_getc_1 (FILE *fp)
 	}
     }
 
+  if (ch >= 255)
+    {
+      /* Readline doesn't understand non-ascii curses keys, filter
+	 them out.  */
+      return 0;
+    }
+
   return ch;
 }
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 4e75a66a00e..a51e7b9f6da 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -498,14 +498,11 @@ tui_resize_all (void)
   height_diff = screenheight - tui_term_height ();
   if (height_diff || width_diff)
     {
-      struct tui_win_info *win_with_focus = tui_win_with_focus ();
-
 #ifdef HAVE_RESIZE_TERM
       resize_term (screenheight, screenwidth);
 #endif      
       /* Turn keypad off while we resize.  */
-      if (win_with_focus != TUI_CMD_WIN)
-	keypad (TUI_CMD_WIN->handle.get (), FALSE);
+      keypad (TUI_CMD_WIN->handle.get (), FALSE);
       tui_update_gdb_sizes ();
       tui_set_term_height_to (screenheight);
       tui_set_term_width_to (screenwidth);
@@ -515,10 +512,8 @@ tui_resize_all (void)
       erase ();
       clearok (curscr, TRUE);
       tui_apply_current_layout ();
-      /* Turn keypad back on, unless focus is in the command
-	 window.  */
-      if (win_with_focus != TUI_CMD_WIN)
-	keypad (TUI_CMD_WIN->handle.get (), TRUE);
+      /* Turn keypad back on.  */
+      keypad (TUI_CMD_WIN->handle.get (), TRUE);
     }
 }
 
@@ -703,7 +698,6 @@ tui_set_focus_command (const char *arg, int from_tty)
     error (_("Window \"%s\" is not visible"), arg);
 
   tui_set_win_focus_to (win_info);
-  keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
   printf_filtered (_("Focus set to %s window.\n"),
 		   tui_win_with_focus ()->name ());
 }
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 529fc62c9ac..5f0c87c05e1 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -179,10 +179,7 @@ tui_rl_other_window (int count, int key)
 
   win_info = tui_next_win (tui_win_with_focus ());
   if (win_info)
-    {
-      tui_set_win_focus_to (win_info);
-      keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
-    }
+    tui_set_win_focus_to (win_info);
   return 0;
 }
 

base-commit: ad9cc2097049a04dc6fa0a593a59f4a3b4807c6f
-- 
2.26.2

Re: [PATCH v3] Make the TUI command window support the mouse

[Hannes Domani]

 Am Sonntag, 13. Juni 2021, 19:26:00 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:

> On 2021-06-13 2:04 p.m., Hannes Domani wrote:
> >  Am Sonntag, 13. Juni 2021, 04:46:15 MESZ hat Pedro Alves <pedro@palves.net> Folgendes geschrieben:
> >
> > This is great, thank you for doing this.
> >
> > I just tried it, and I had just one problem, see below.
> >
>
> >> +      /* For the standard keys, we can find them efficiently in the
> >> +    key_xxx macros, defined by ncurses.  We could also hardcode
> >> +    sequences readline understands, and/or use rl_get_termcap.
> >> +    See readline/readline.c:bind_arrow_keys_internal for
> >> +    hardcoded sequences.  */
> >> +      switch (ch)
> >> +    {
> >> +    case KEY_NPAGE: /* page down */
> >> +      return start_sequence (key_npage);
> >> +    case KEY_PPAGE: /* page up */
> >> +      return start_sequence (key_ppage);
> >> +    case KEY_DOWN:
> >> +      return start_sequence (key_down);
> >> +    case KEY_UP:
> >> +      return start_sequence (key_up);
> >> +    case KEY_RIGHT:
> >> +      return start_sequence (key_right);
> >> +    case KEY_LEFT:
> >> +      return start_sequence (key_left);
> >> +    case KEY_HOME:
> >> +      return start_sequence (key_home);
> >> +    case KEY_END:
> >> +      return start_sequence (key_end);
> >> +    case KEY_DC: /* del */
> >> +      return start_sequence (key_dc);
> >> +    case KEY_IC: /* ins */
> >> +      return start_sequence (key_ic);
> >> +    }
> >
> > PDcurses doesn't have these key_npage/key_ppage/key_down/... variables,
> > so I had to use the hardcoded sequences as you described.
> > This is working for the arrow keys and home/end, and for del I used '\004'
> > instead, but I didn't see any for page up/down and ins.
>
> Ack.  \004 is ctrl-d  ('d' & 0x1f).  I'm a bit wary of hardcoding it,
> since I imagine there might be some users out where that have bound
> ctrl-d to some other action in their inputrc.
>
> readline's bind_arrow_keys_internal has:
>
> #if defined (__MINGW32__)
>   rl_bind_keyseq_if_unbound ("\340H", rl_get_previous_history);
>   rl_bind_keyseq_if_unbound ("\340P", rl_get_next_history);
>   rl_bind_keyseq_if_unbound ("\340M", rl_forward_char);
>   rl_bind_keyseq_if_unbound ("\340K", rl_backward_char);
>   rl_bind_keyseq_if_unbound ("\340G", rl_beg_of_line);
>   rl_bind_keyseq_if_unbound ("\340O", rl_end_of_line);
>   rl_bind_keyseq_if_unbound ("\340S", rl_delete);
>   rl_bind_keyseq_if_unbound ("\340R", rl_overwrite_mode);
>
>   /* These may or may not work because of the embedded NUL. */
>   rl_bind_keyseq_if_unbound ("\\000H", rl_get_previous_history);
>   rl_bind_keyseq_if_unbound ("\\000P", rl_get_next_history);
>   rl_bind_keyseq_if_unbound ("\\000M", rl_forward_char);
>   rl_bind_keyseq_if_unbound ("\\000K", rl_backward_char);
>   rl_bind_keyseq_if_unbound ("\\000G", rl_beg_of_line);
>   rl_bind_keyseq_if_unbound ("\\000O", rl_end_of_line);
>   rl_bind_keyseq_if_unbound ("\\000S", rl_delete);
>   rl_bind_keyseq_if_unbound ("\\000R", rl_overwrite_mode);
> #endif
>
> So would "\340S" and "\340R" work instead?  See attached patch.
>
> >
> > I'm not sure if it's even a problem for page up/down, they seem to not be used
> > by readline, but I couldn't make ins work.
>
> Ins works for me on konsole, and on a linux virtual console.  It does not work
> on xterm and rxvt.  Nor on konsole + ssh.  Nor under screen.  I confirmed that
> I get the same key sequence for the insert key on all those cases (run "cat" + press
> the key).  It's the same with bash and gdb in non-tui mode.  It's not specific to this
> patch.  I would guess readline is somehow finding that the terminal does not support
> overwrite mode.
>
> >
> > And rl_get_termcap returned for everything except "le" NULL, but I assume
> > that's just how it is on Windows.
>
> Unfortunate.
>
> Here's a new version.  Does this one work for you?

Yes, with this everything works, thanks.


Hannes

Re: [PATCH v3] Make the TUI command window support the mouse

[Pedro Alves]

On 2021-06-13 6:55 p.m., Hannes Domani wrote:

>> Here's a new version.  Does this one work for you?
> 
> Yes, with this everything works, thanks.

Awesome!  I'll wait for tomorrow for merging, in case Tromey or someone else
wants to comment.

Pedro Alves

[PUSHED v4] Make the TUI command window support the mouse

[Pedro Alves]

On 2021-06-13 6:59 p.m., Pedro Alves wrote:
> On 2021-06-13 6:55 p.m., Hannes Domani wrote:
> 
>>> Here's a new version.  Does this one work for you?
>>
>> Yes, with this everything works, thanks.
> 
> Awesome!  I'll wait for tomorrow for merging, in case Tromey or someone else
> wants to comment.

I've merged this now.  I did a couple small extra tweaks before pushing:

- The NCURSES_MOUSE_VERSION wrapping is now outside of
  tui_dispatch_mouse_event instead of inside.  

before:

+static void
+tui_dispatch_mouse_event ()
+{
+#ifdef NCURSES_MOUSE_VERSION

now:

+#ifdef NCURSES_MOUSE_VERSION
+
+static void
+tui_dispatch_mouse_event ()


I was doing it inside in the previous version because I experimented with always
unconditionally calling the function.  Then I reverted it before posting and
missed moving the #ifdef outside the function.

- I'm saying non-8-bit instead of non-ascii.  And fixed an off by one...

old:

+  if (ch >= 255)
+    {
+      /* Readline doesn't understand non-ascii curses keys, filter
+	 them out.  */
+      return 0;
+    }

new:

+  if (ch > 0xff)
+    {
+      /* Readline doesn't understand non-8-bit curses keys, filter
+        them out.  */
+      return 0;
+    }

Here's what I pushed.

From 82a5082ed3f00b8d7bd1e3810c6eeb4351e35286 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Thu, 17 Jun 2021 11:57:56 +0100
Subject: [PATCH] Make the TUI command window support the mouse

Currently, when the focus is on the command window, we disable the
keypad.  This means that when the command window has the focus, keys
such as up/down/home/end etc. are not processed by curses, and their
escape sequences go straight to readline.

A side effect of disabling keypad mode is that wgetch no longer
processes mouse escape sequences, with the end result being the mouse
doesn't work, and worse, the raw mouse escape sequences are printed on
the terminal.

This commit makes the TUI command window support the mouse as well, by
always enabling the keypad, and then to avoid losing support for
up/down browsing the command history, home/end/left/right moving the
cursor position, etc., we forward those keys as raw escape sequences
to readline.  Note we don't make an effort to pass down to readline
all keys returned by curses, only the common ones that readline
understands by default.  Given users can specify their own readline
bindings (inputrc file, bind utility), this approach is good in
practice, though not 100% transparent or perfect.

Note that the patch makes it so that CTLC-L is always passed to
readline even if the command window does not have the focus.  It was
simpler to implement that way, and it just seems correct to me.  I
don't know of a reason we shouldn't do that.

The patch improves the TUI behavior in a related way.  Now we can pass
special keys to readline irrespective of which window has the focus.
First, we try to dispatch the key to a window, via
tui_displatch_ctrl_char.  If the key is dispatched, then we don't pass
it to readline.  E.g., pressing "up" when you have the source window
in focus results in scrolling the source window, and nothing else.  If
however, you press ctrl-del instead, that results in killing the next
word in the command window, no matter which window has has focus.
Before, it would only work if you had the command window in focus.
Similarly, ctrl-left/ctrl-right to move between words, etc.

Similarly, the previous spot where we handled mouse events was
incorrect.  It was never reached if the window with focus can't
scroll, which is the case for the command window.  Mouse scrolling
affects the window under the mouse cursor, not the window in focus.
We now always try to dispatch mouse events.

One last bit in the patch -- now if we don't recognize the non-8-bit
curses key, then we don't pass it down to readline at all.  Before
that would result in bogus characters in the input line.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	* tui/tui-io.c (tui_dispatch_mouse_event): New, factored out from
	...
	(tui_dispatch_ctrl_char): ... this.  Move CTRL-L handling to
	tui_getc_1.
	(cur_seq, start_sequence): New.
	(tui_getc_1): Pass key escape sequences for curses control keys to
	readline.  Handle mouse and ctrl-l here.
	(tui_resize_all): Disable/reenable the keypad if the command
	window has the focus too.
	* tui/tui-win.c (tui_set_focus_command): Don't change keypad
	setting.
	* tui/tui.c (tui_rl_other_window): Don't change keypad setting.

Change-Id: Ie0a7d849943cfb47f4a6589e1c73341563740fa9
---
 gdb/ChangeLog     |  15 ++++
 gdb/tui/tui-io.c  | 206 +++++++++++++++++++++++++++++++++++++---------
 gdb/tui/tui-win.c |  12 +--
 gdb/tui/tui.c     |   5 +-
 4 files changed, 186 insertions(+), 52 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2444773bde9..497d9f6aee4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2021-06-17  Pedro Alves  <pedro@palves.net>
+
+	* tui/tui-io.c (tui_dispatch_mouse_event): New, factored out from
+	...
+	(tui_dispatch_ctrl_char): ... this.  Move CTRL-L handling to
+	tui_getc_1.
+	(cur_seq, start_sequence): New.
+	(tui_getc_1): Pass key escape sequences for curses control keys to
+	readline.  Handle mouse and ctrl-l here.
+	(tui_resize_all): Disable/reenable the keypad if the command
+	window has the focus too.
+	* tui/tui-win.c (tui_set_focus_command): Don't change keypad
+	setting.
+	* tui/tui.c (tui_rl_other_window): Don't change keypad setting.
+
 2021-06-16  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* silent-rules.mk (ECHO_CCLD, ECHO_AR, ECHO_RANLIB): New.
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 7df0e2f1bd3..bd443dc0c0c 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -946,6 +946,42 @@ tui_initialize_io (void)
 #endif
 }
 
+/* Dispatch the correct tui function based upon the mouse event.  */
+
+#ifdef NCURSES_MOUSE_VERSION
+
+static void
+tui_dispatch_mouse_event ()
+{
+  MEVENT mev;
+  if (getmouse (&mev) != OK)
+    return;
+
+  for (tui_win_info *wi : all_tui_windows ())
+    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
+	&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
+      {
+	if ((mev.bstate & BUTTON1_CLICKED) != 0
+	    || (mev.bstate & BUTTON2_CLICKED) != 0
+	    || (mev.bstate & BUTTON3_CLICKED) != 0)
+	  {
+	    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
+	      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
+			 : 3);
+	    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
+	  }
+#ifdef BUTTON5_PRESSED
+	else if ((mev.bstate & BUTTON4_PRESSED) != 0)
+	  wi->backward_scroll (3);
+	else if ((mev.bstate & BUTTON5_PRESSED) != 0)
+	  wi->forward_scroll (3);
+#endif
+	break;
+      }
+}
+
+#endif
+
 /* Dispatch the correct tui function based upon the control
    character.  */
 static unsigned int
@@ -953,10 +989,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
 {
   struct tui_win_info *win_info = tui_win_with_focus ();
 
-  /* Handle the CTRL-L refresh for each window.  */
-  if (ch == '\f')
-    tui_refresh_all_win ();
-
   /* If no window has the focus, or if the focus window can't scroll,
      just pass the character through.  */
   if (win_info == NULL || !win_info->can_scroll ())
@@ -984,39 +1016,6 @@ tui_dispatch_ctrl_char (unsigned int ch)
     case KEY_LEFT:
       win_info->right_scroll (1);
       break;
-#ifdef NCURSES_MOUSE_VERSION
-    case KEY_MOUSE:
-	{
-	  MEVENT mev;
-	  if (getmouse (&mev) != OK)
-	    break;
-
-	  for (tui_win_info *wi : all_tui_windows ())
-	    if (mev.x > wi->x && mev.x < wi->x + wi->width - 1
-		&& mev.y > wi->y && mev.y < wi->y + wi->height - 1)
-	      {
-		if ((mev.bstate & BUTTON1_CLICKED) != 0
-		    || (mev.bstate & BUTTON2_CLICKED) != 0
-		    || (mev.bstate & BUTTON3_CLICKED) != 0)
-		  {
-		    int button = (mev.bstate & BUTTON1_CLICKED) != 0 ? 1
-		      :         ((mev.bstate & BUTTON2_CLICKED) != 0 ? 2
-				 : 3);
-		    wi->click (mev.x - wi->x - 1, mev.y - wi->y - 1, button);
-		  }
-#ifdef BUTTON5_PRESSED
-		else if ((mev.bstate & BUTTON4_PRESSED) != 0)
-		  wi->backward_scroll (3);
-		else if ((mev.bstate & BUTTON5_PRESSED) != 0)
-		  wi->forward_scroll (3);
-#endif
-		break;
-	      }
-	}
-      break;
-#endif
-    case '\f':
-      break;
     default:
       /* We didn't recognize the character as a control character, so pass it
 	 through.  */
@@ -1067,6 +1066,24 @@ tui_inject_newline_into_command_window ()
     }
 }
 
+/* If we're passing an escape sequence to readline, this points to a
+   string holding the remaining characters of the sequence to pass.
+   We advance the pointer one character at a time until '\0' is
+   reached.  */
+static const char *cur_seq = nullptr;
+
+/* Set CUR_SEQ to point at the current sequence to pass to readline,
+   setup to call the input handler again so we complete the sequence
+   shortly, and return the first character to start the sequence.  */
+
+static int
+start_sequence (const char *seq)
+{
+  call_stdin_event_handler_again_p = 1;
+  cur_seq = seq + 1;
+  return seq[0];
+}
+
 /* Main worker for tui_getc.  Get a character from the command window.
    This is called from the readline package, but wrapped in a
    try/catch by tui_getc.  */
@@ -1084,11 +1101,115 @@ tui_getc_1 (FILE *fp)
   tui_readline_output (0, 0);
 #endif
 
-  ch = gdb_wgetch (w);
+  /* We enable keypad mode so that curses's wgetch processes mouse
+     escape sequences.  In keypad mode, wgetch also processes the
+     escape sequences for keys such as up/down etc. and returns KEY_UP
+     / KEY_DOWN etc.  When we have the focus on the command window
+     though, we want to pass the raw up/down etc. escape codes to
+     readline so readline understands them.  */
+  if (cur_seq != nullptr)
+    {
+      ch = *cur_seq++;
+
+      /* If we've reached the end of the string, we're done with the
+	 sequence.  Otherwise, setup to get back here again for
+	 another character.  */
+      if (*cur_seq == '\0')
+	cur_seq = nullptr;
+      else
+	call_stdin_event_handler_again_p = 1;
+      return ch;
+    }
+  else
+    ch = gdb_wgetch (w);
 
   /* Handle prev/next/up/down here.  */
   ch = tui_dispatch_ctrl_char (ch);
-  
+
+#ifdef NCURSES_MOUSE_VERSION
+  if (ch == KEY_MOUSE)
+    {
+      tui_dispatch_mouse_event ();
+      return 0;
+    }
+#endif
+
+  /* Translate curses keys back to escape sequences so that readline
+     can understand them.  We do this irrespective of which window has
+     the focus.  If e.g., we're focused on a non-command window, then
+     the up/down keys will already have been filtered by
+     tui_dispatch_ctrl_char.  Keys that haven't been intercepted will
+     be passed down to readline.  */
+  if (current_ui->command_editing)
+    {
+      /* For the standard arrow keys + home/end, hardcode sequences
+	 readline understands.  See bind_arrow_keys_internal in
+	 readline/readline.c.  */
+      switch (ch)
+	{
+	case KEY_UP:
+	  return start_sequence ("\033[A");
+	case KEY_DOWN:
+	  return start_sequence ("\033[B");
+	case KEY_RIGHT:
+	  return start_sequence ("\033[C");
+	case KEY_LEFT:
+	  return start_sequence ("\033[D");
+	case KEY_HOME:
+	  return start_sequence ("\033[H");
+	case KEY_END:
+	  return start_sequence ("\033[F");
+
+	/* del and ins are unfortunately not hardcoded in readline for
+	   all systems.  */
+
+	case KEY_DC: /* del */
+#ifdef __MINGW32__
+	  return start_sequence ("\340S");
+#else
+	  return start_sequence ("\033[3~");
+#endif
+
+	case KEY_IC: /* ins */
+#if defined __MINGW32__
+	  return start_sequence ("\340R");
+#else
+	  return start_sequence ("\033[2~");
+#endif
+	}
+
+      /* Keycodes above KEY_MAX are not garanteed to be stable.
+	 Compare keyname instead.  */
+      if (ch >= KEY_MAX)
+	{
+	  auto name = gdb::string_view (keyname (ch));
+
+	  /* The following sequences are hardcoded in readline as
+	     well.  */
+
+	  /* ctrl-arrow keys */
+	  if (name == "kLFT5") /* ctrl-left */
+	    return start_sequence ("\033[1;5D");
+	  else if (name == "kRIT5") /* ctrl-right */
+	    return start_sequence ("\033[1;5C");
+	  else if (name == "kDC5") /* ctrl-del */
+	    return start_sequence ("\033[3;5~");
+
+	  /* alt-arrow keys */
+	  else if (name == "kLFT3") /* alt-left */
+	    return start_sequence ("\033[1;3D");
+	  else if (name == "kRIT3") /* alt-right */
+	    return start_sequence ("\033[1;3C");
+	}
+    }
+
+  /* Handle the CTRL-L refresh for each window.  */
+  if (ch == '\f')
+    {
+      tui_refresh_all_win ();
+      return ch;
+    }
+
   if (ch == KEY_BACKSPACE)
     return '\b';
 
@@ -1118,6 +1239,13 @@ tui_getc_1 (FILE *fp)
 	}
     }
 
+  if (ch > 0xff)
+    {
+      /* Readline doesn't understand non-8-bit curses keys, filter
+	 them out.  */
+      return 0;
+    }
+
   return ch;
 }
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 4e75a66a00e..a51e7b9f6da 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -498,14 +498,11 @@ tui_resize_all (void)
   height_diff = screenheight - tui_term_height ();
   if (height_diff || width_diff)
     {
-      struct tui_win_info *win_with_focus = tui_win_with_focus ();
-
 #ifdef HAVE_RESIZE_TERM
       resize_term (screenheight, screenwidth);
 #endif      
       /* Turn keypad off while we resize.  */
-      if (win_with_focus != TUI_CMD_WIN)
-	keypad (TUI_CMD_WIN->handle.get (), FALSE);
+      keypad (TUI_CMD_WIN->handle.get (), FALSE);
       tui_update_gdb_sizes ();
       tui_set_term_height_to (screenheight);
       tui_set_term_width_to (screenwidth);
@@ -515,10 +512,8 @@ tui_resize_all (void)
       erase ();
       clearok (curscr, TRUE);
       tui_apply_current_layout ();
-      /* Turn keypad back on, unless focus is in the command
-	 window.  */
-      if (win_with_focus != TUI_CMD_WIN)
-	keypad (TUI_CMD_WIN->handle.get (), TRUE);
+      /* Turn keypad back on.  */
+      keypad (TUI_CMD_WIN->handle.get (), TRUE);
     }
 }
 
@@ -703,7 +698,6 @@ tui_set_focus_command (const char *arg, int from_tty)
     error (_("Window \"%s\" is not visible"), arg);
 
   tui_set_win_focus_to (win_info);
-  keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
   printf_filtered (_("Focus set to %s window.\n"),
 		   tui_win_with_focus ()->name ());
 }
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 529fc62c9ac..5f0c87c05e1 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -179,10 +179,7 @@ tui_rl_other_window (int count, int key)
 
   win_info = tui_next_win (tui_win_with_focus ());
   if (win_info)
-    {
-      tui_set_win_focus_to (win_info);
-      keypad (TUI_CMD_WIN->handle.get (), win_info != TUI_CMD_WIN);
-    }
+    tui_set_win_focus_to (win_info);
   return 0;
 }
 

base-commit: 3478a63d7ed68d666f842f5b8fb5bdade619c817
-- 
2.26.2