Patches to improve the gdbtui user experience

Patches to improve the gdbtui user experience

[Jan Holesovsky]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

Hi,

I've already sent these patches once, but got no response - could you please 
have a look, and apply if OK?  They are more or less trivial, but especially 
the single-key-history.diff and fix-prompt.diff improve the user experience 
of single key mode a lot.  Comments appreciated ;-)

ChangeLog for the changes:

2007-05-21  Jan Holesovsky  <kendy@suse.cz>

	* tui/tui.c: SingleKey binding for stepi and nexti; don't quit
	  SingleKey by 'q' when it's activated by a nontrivial ctrl+x
	  combination - the user is smart enough to exit this mode.
	* tui/tui-win.c: Fix silly typos in help.
	* tui/tui-io.c: Display prompts like ---Type <return>... even in
	  SingleKey mode, and be able to react there.
	* tui/tui-command.c: When switched temporarily from the SingleKey mode
	  to command mode, bind [Up] and [Down] keys to readline history; it's
	  safe because we know that the user wants to type.

Thank you a lot in advance!

Regards,
Jan

[-- Attachment #2: better-keys.diff --]
[-- Type: text/x-diff, Size: 955 bytes --]

diff -pur gdb-6.6.orig/gdb/tui/tui.c gdb-6.6/gdb/tui/tui.c
--- gdb-6.6.orig/gdb/tui/tui.c	2005-12-23 11:10:03.000000000 -0800
+++ gdb-6.6/gdb/tui/tui.c	2007-05-16 14:21:11.000000000 -0700
@@ -75,12 +75,14 @@ static const struct tui_char_command tui
   { 'c', "continue" },
   { 'd', "down" },
   { 'f', "finish" },
+  { 'i', "stepi" },
   { 'n', "next" },
   { 'r', "run" },
   { 's', "step" },
   { 'u', "up" },
   { 'v', "info locals" },
   { 'w', "where" },
+  { 'x', "nexti" },
   { 0, 0 },
 };
 
@@ -349,7 +351,6 @@ tui_initialize_readline (void)
   rl_bind_key_in_map ('2', tui_rl_change_windows, tui_ctlx_keymap);
   rl_bind_key_in_map ('o', tui_rl_other_window, emacs_ctlx_keymap);
   rl_bind_key_in_map ('o', tui_rl_other_window, tui_ctlx_keymap);
-  rl_bind_key_in_map ('q', tui_rl_next_keymap, tui_keymap);
   rl_bind_key_in_map ('s', tui_rl_next_keymap, emacs_ctlx_keymap);
   rl_bind_key_in_map ('s', tui_rl_next_keymap, tui_ctlx_keymap);
 }

[-- Attachment #3: fix-help.diff --]
[-- Type: text/x-diff, Size: 666 bytes --]

diff -pur gdb-6.6.orig/gdb/tui/tui-win.c gdb-6.6/gdb/tui/tui-win.c
--- gdb-6.6.orig/gdb/tui/tui-win.c	2006-02-14 11:05:40.000000000 -0800
+++ gdb-6.6/gdb/tui/tui-win.c	2007-05-14 18:51:40.000000000 -0700
@@ -389,10 +389,10 @@ Usage: + [win] [n]\n"));
 Scroll window backward.\n\
 Usage: - [win] [n]\n"));
   add_com ("<", class_tui, tui_scroll_left_command, _("\
-Scroll window forward.\n\
+Scroll window left.\n\
 Usage: < [win] [n]\n"));
   add_com (">", class_tui, tui_scroll_right_command, _("\
-Scroll window backward.\n\
+Scroll window right.\n\
 Usage: > [win] [n]\n"));
   if (xdb_commands)
     add_com ("w", class_xdb, tui_xdb_set_win_height_command, _("\

[-- Attachment #4: single-key-history.diff --]
[-- Type: text/x-diff, Size: 1068 bytes --]

diff -pur gdb-6.6.orig/gdb/tui/tui-command.c gdb-6.6/gdb/tui/tui-command.c
--- gdb-6.6.orig/gdb/tui/tui-command.c	2005-12-23 11:10:02.000000000 -0800
+++ gdb-6.6/gdb/tui/tui-command.c	2007-05-15 14:46:12.000000000 -0700
@@ -32,6 +32,7 @@
 #include "gdb_curses.h"
 #include "gdb_string.h"
 
+#include <readline/readline.h>
 
 /*****************************************
 ** STATIC LOCAL FUNCTIONS FORWARD DECLS    **
@@ -112,11 +113,23 @@ tui_dispatch_ctrl_char (unsigned int ch)
 	  break;
 	case KEY_DOWN:
 	case KEY_SF:
-	  tui_scroll_forward (win_info, 1);
+	  if (tui_current_key_mode == TUI_ONE_COMMAND_MODE)
+	    {
+	      rl_get_next_history (1, 0);
+	      rl_redisplay ();
+	    }
+	  else
+	    tui_scroll_forward (win_info, 1);
 	  break;
 	case KEY_UP:
 	case KEY_SR:
-	  tui_scroll_backward (win_info, 1);
+	  if (tui_current_key_mode == TUI_ONE_COMMAND_MODE)
+	    {
+	      rl_get_previous_history (1, 0);
+	      rl_redisplay ();
+	    }
+	  else
+	    tui_scroll_backward (win_info, 1);
 	  break;
 	case KEY_RIGHT:
 	  tui_scroll_left (win_info, 1);

[-- Attachment #5: fix-prompt.diff --]
[-- Type: text/x-diff, Size: 867 bytes --]

diff -pur gdb-6.6.orig/gdb/tui/tui-io.c gdb-6.6/gdb/tui/tui-io.c
--- gdb-6.6.orig/gdb/tui/tui-io.c	2005-12-23 11:10:02.000000000 -0800
+++ gdb-6.6/gdb/tui/tui-io.c	2007-05-16 07:59:10.000000000 -0700
@@ -211,11 +211,17 @@ tui_redisplay_readline (void)
   if (tui_current_key_mode == TUI_ONE_COMMAND_MODE && rl_end == 0)
     tui_set_key_mode (TUI_SINGLE_KEY_MODE);
 
+  /* Do display prompts like ---Type <return>... even in single command
+     mode, and be able to react on such requests. */
+  prompt = tui_rl_saved_prompt;
   if (tui_current_key_mode == TUI_SINGLE_KEY_MODE)
-    prompt = "";
-  else
-    prompt = tui_rl_saved_prompt;
-  
+    {
+      if (!strcmp (tui_rl_saved_prompt, get_prompt()))
+        prompt = "";
+      else
+        tui_set_key_mode (TUI_ONE_COMMAND_MODE);
+    }
+
   c_pos = -1;
   c_line = -1;
   w = TUI_CMD_WIN->generic.handle;

Re: Patches to improve the gdbtui user experience

[Daniel Jacobowitz]

On Fri, Sep 14, 2007 at 01:58:24PM +0200, Jan Holesovsky wrote:
> Hi,
> 
> I've already sent these patches once, but got no response - could you please 
> have a look, and apply if OK?  They are more or less trivial, but especially 
> the single-key-history.diff and fix-prompt.diff improve the user experience 
> of single key mode a lot.  Comments appreciated ;-)

Is there anyone reading gdb-patches who uses gdbtui and can comment on
these?  I put off looking at them, last time, since I don't.


-- 
Daniel Jacobowitz
CodeSourcery

Re: Patches to improve the gdbtui user experience

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]

Jan Holesovsky wrote:
> 
> I've already sent these patches once, but got no response - could you please 
> have a look, and apply if OK?  They are more or less trivial, but especially 
> the single-key-history.diff and fix-prompt.diff improve the user experience 
> of single key mode a lot.  Comments appreciated ;-)
> 

I use tui mode often, but single key mode, not so often.  Anyway, see my
comments below, but note that I can't approve patches.

> ChangeLog for the changes:
> 
> 2007-05-21  Jan Holesovsky  <kendy@suse.cz>
> 
> 	* tui/tui.c: SingleKey binding for stepi and nexti; 

This seems useful indeed.  Thanks!

> don't quit
> 	  SingleKey by 'q' when it's activated by a nontrivial ctrl+x
  > 	  combination - the user is smart enough to exit this mode.

If the only command gdb knows starting with 'q' is "quit", what
is the use?  What is it preventing you to type?  Maybe
a target/host specific command?  At least on cygwin 'q' is not
ambiguous.  On the other hand, if you're in in single key mode, and
want to type "quit", you have to type "qquit", since the first 'q' is
eaten, else you'll end up typing "uit" :)  I guess it's a matter of
personal taste either way.

> 	* tui/tui-win.c: Fix silly typos in help.

This is trivial indeed.

> 	* tui/tui-io.c: Display prompts like ---Type <return>... even in
> 	  SingleKey mode, and be able to react there.

Seems like a hack to me.  What about not setting TUI_SINGLE_KEY_MODE
in the first place, if gdb is in the middle of a possibly length
command?  The attached works for me, but I didn't regtest it.  Is
this usage of immediate_quit ok?

> 	* tui/tui-command.c: When switched temporarily from the SingleKey mode
> 	  to command mode, bind [Up] and [Down] keys to readline history; it's
> 	  safe because we know that the user wants to type.

In single key mode, I can start to type 'b' for a breakpoint, and
then decide I want to scroll to see at which line I would want to place the
breakpoint at using up/down.  If I want access to the history, I can either
use c-p/c-n, or use c-x o to switch focus to the command line, and
then use up/down.  With your patch, if I already started typing 'b',
I'll have no way to scroll, other than deleting what I've already
typed, will I?

Cheers,
Pedro Alves


[-- Attachment #2: fix-prompt.diff --]
[-- Type: text/x-diff, Size: 2127 bytes --]

	* tui/tui.c (tui_rl_command_mode): Update comment.
	(tui_rl_startup_hook): Don't change to single key mode, if
	immediate_quit is set.
	* tui/tui-io.c (tui_redisplay_readline): Likewise.

---
 gdb/tui/tui-io.c |    8 ++++++--
 gdb/tui/tui.c    |    9 +++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

Index: src/gdb/tui/tui-io.c
===================================================================
--- src.orig/gdb/tui/tui-io.c	2007-09-03 22:27:56.000000000 +0100
+++ src/gdb/tui/tui-io.c	2007-09-15 17:35:18.000000000 +0100
@@ -209,9 +209,13 @@ tui_redisplay_readline (void)
 
   /* Detect when we temporarily left SingleKey and now the readline
      edit buffer is empty, automatically restore the SingleKey
-     mode.  */
+     mode, unless gdb is in the middle of executing a command,
+     possibly expecting user interaction.  */
   if (tui_current_key_mode == TUI_ONE_COMMAND_MODE && rl_end == 0)
-    tui_set_key_mode (TUI_SINGLE_KEY_MODE);
+    {
+      if (!immediate_quit)
+	tui_set_key_mode (TUI_SINGLE_KEY_MODE);
+    }
 
   if (tui_current_key_mode == TUI_SINGLE_KEY_MODE)
     prompt = "";
Index: src/gdb/tui/tui.c
===================================================================
--- src.orig/gdb/tui/tui.c	2007-09-03 22:27:58.000000000 +0100
+++ src/gdb/tui/tui.c	2007-09-15 17:33:46.000000000 +0100
@@ -255,7 +255,9 @@ tui_rl_command_key (int count, int key)
 /* TUI readline command.
    Temporarily leave the TUI SingleKey mode to allow editing
    a gdb command with the normal readline.  Once the command
-   is executed, the TUI SingleKey mode is installed back.  */
+   is executed, the TUI SingleKey mode is installed back,
+   unless gdb is in the middle of executing a command, possibly
+   expecting user interaction.  */
 static int
 tui_rl_command_mode (int count, int key)
 {
@@ -285,7 +287,10 @@ tui_rl_startup_hook (void)
 {
   rl_already_prompted = 1;
   if (tui_current_key_mode != TUI_COMMAND_MODE)
-    tui_set_key_mode (TUI_SINGLE_KEY_MODE);
+    {
+      if (!immediate_quit)
+	tui_set_key_mode (TUI_SINGLE_KEY_MODE);
+    }
   tui_redisplay_readline ();
   return 0;
 }

Re: Patches to improve the gdbtui user experience

[Pedro Alves]

Pedro Alves wrote:
> Jan Holesovsky wrote:
>> don't quit
>>       SingleKey by 'q' when it's activated by a nontrivial ctrl+x
>  >       combination - the user is smart enough to exit this mode.
> 
> If the only command gdb knows starting with 'q' is "quit", what
> is the use?  What is it preventing you to type?  Maybe
> a target/host specific command?  At least on cygwin 'q' is not
> ambiguous.  On the other hand, if you're in in single key mode, and
> want to type "quit", you have to type "qquit", since the first 'q' is
> eaten, else you'll end up typing "uit" :)  I guess it's a matter of
> personal taste either way.
> 

Let me just add a note to say that in my opinion, entering
'one command mode' with any other key (that is, a key that
has no binding in 'single key mode'), is broken.  One has to
remember which keys are bound, and which aren't - that makes
it too easy to accidently issue some command you didn't
want to.  Something like pressing <space> to enter 'one
command mode' would make much more sense to me - principle of
least surprise.

eg:
I want to type 'info something', and I currently can because
'i' is not bound to anything.  With your patch, 'i' would
stepi.  There ya go, I just stepped accidently, possibly over a
jmp, when I didn't want to.  I might have wanted to 'return 0',
for example.

Cheers,
Pedro Alves