[2/2,gdb/tui] Delete line buffer when switching to singlekey

Message ID 20230606150333.5206-2-tdevries@suse.de
State Committed
Headers
Series [1/2,gdb/testsuite] Add test-case gdb.tui/single-key.exp |

Commit Message

Tom de Vries June 6, 2023, 3:03 p.m. UTC
  Say we're in TUI mode, and type "sun":
...
(gdb) sun
...

After switching to SingleKey mode using C-x s, we have just:
...
sun
...

After typing "d", we get:
...
sun
Undefined command: "sundown".  Try "help".
...

The SingleKey "d" is supposed run the "down" command.

Fix this by clearing the readline line buffer when switching to SingleKey
mode.

Tested on x86_64-linux.

PR tui/30522
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30522
---
 gdb/testsuite/gdb.tui/single-key-2.exp | 40 ++++++++++++++++++++++++++
 gdb/tui/tui.c                          |  7 +++++
 2 files changed, 47 insertions(+)
 create mode 100644 gdb/testsuite/gdb.tui/single-key-2.exp
  

Comments

Tom Tromey June 9, 2023, 2:16 p.m. UTC | #1
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> The SingleKey "d" is supposed run the "down" command.

Tom> Fix this by clearing the readline line buffer when switching to SingleKey
Tom> mode.

I was wondering if saving and restoring the partial text would be
better, but honestly this is better than the status quo, and it's not
clear to me that people switch in and out of SingleKey so much that this
kind of detail would matter.

So, seems good.

Reviewed-By: Tom Tromey <tom@tromey.com>

Tom
  
Tom de Vries June 9, 2023, 2:37 p.m. UTC | #2
On 6/9/23 16:16, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> The SingleKey "d" is supposed run the "down" command.
> 
> Tom> Fix this by clearing the readline line buffer when switching to SingleKey
> Tom> mode.
> 
> I was wondering if saving and restoring the partial text would be
> better, but honestly this is better than the status quo, and it's not
> clear to me that people switch in and out of SingleKey so much that this
> kind of detail would matter.
> 

Thanks for the review.

We can compare behaviour with C-x a:
- type foo on gdb prompt
- now enter TUI by typing C-x a
- exit TUI again by typing the same
- we see "(gdb) foo" but that's just an artifact, see PR30523
- we type space three times and backspace to overwrite the artifact
- we can now type C-x y, and we get again "(gdb) foo", this time
   indeed in the readline buffer

So, entering TUI with C-x a kills the prompt line and puts it in the 
kill ring.  Exiting TUI does not restore it.

I'm not 100% convinced that the choice of kill rather than delete is 
intentional either.  The comment says:
...
   /* Clear the readline in case switching occurred in middle of 

      something.  */
   if (rl_end)
     rl_kill_text (0, rl_end);
...

So, I opted for a simple delete.

Thanks,
- Tom

> So, seems good.
> 
> Reviewed-By: Tom Tromey <tom@tromey.com>
> 
> Tom
  

Patch

diff --git a/gdb/testsuite/gdb.tui/single-key-2.exp b/gdb/testsuite/gdb.tui/single-key-2.exp
new file mode 100644
index 00000000000..5bde522ec20
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/single-key-2.exp
@@ -0,0 +1,40 @@ 
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+tuiterm_env
+
+Term::clean_restart 24 80
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+set command_win { 0 16 80 8 }
+
+# Send some chars as a partial command.
+send_gdb "sun"
+gdb_assert { [Term::wait_for_region_contents 0 16 80 8 "$gdb_prompt sun"] } \
+    "got sun"
+
+# Enter single-key mode.  That should clear the partial command.
+send_gdb "\030s"
+
+# Send "down command".
+send_gdb "d"
+
+# Regression test for PR tui/30522.  Make sure we issue the command "down"
+# rather than "sundown".
+gdb_assert { [Term::wait_for_region_contents {*}$command_win "No stack\\."] } \
+    "down using single-key after partial command"
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 373d1673e3a..43be8161e4c 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -252,6 +252,13 @@  tui_rl_next_keymap (int notused1, int notused2)
   if (!tui_active)
     tui_rl_switch_mode (0 /* notused */, 0 /* notused */);
 
+  if (rl_end)
+    {
+      rl_end = 0;
+      rl_point = 0;
+      rl_mark = 0;
+    }
+
   tui_set_key_mode (tui_current_key_mode == TUI_COMMAND_MODE
 		    ? TUI_SINGLE_KEY_MODE : TUI_COMMAND_MODE);
   return 0;