[v2] Make the TUI command window support the mouse

Message ID 455337ea-65c3-42b5-0ec0-3c85b85ba054@palves.net
State Superseded
Headers show
Series
  • [v2] Make the TUI command window support the mouse
Related show

Commit Message

Pedro Alves June 13, 2021, 2:46 a.m.
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(-)

Comments

Tom de Vries via Gdb-patches June 13, 2021, 10:35 a.m. | #1
> 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?
Tom de Vries via Gdb-patches June 13, 2021, 1:04 p.m. | #2
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
Pedro Alves June 13, 2021, 5:29 p.m. | #3
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
Tom de Vries via Gdb-patches June 13, 2021, 6:02 p.m. | #4
> 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.
Pedro Alves June 13, 2021, 6:13 p.m. | #5
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

Patch

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;
 }