[1/3] Remove superfluous function key_is_command_char()

Message ID 1420689885-31156-1-git-send-email-patrick@parcs.ath.cx
State New, archived
Headers

Commit Message

Patrick Palka Jan. 8, 2015, 4:04 a.m. UTC
  The function key_is_command_char() is simply a predicate that determines
whether the function tui_dispatch_ctrl_char() will do anything useful.
Since tui_dispatch_ctrl_char() performs the same checks as
key_is_command_char() it is unnecessary to keep key_is_command_char()
around.  This patch removes this useless function and instead
unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
tui_getc().

gdb/ChangeLog:

	* tui/tui-io.c (tui_getc): Don't call key_is_command_char.
	(key_is_command_char): Delete.
---
 gdb/tui/tui-io.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)
  

Comments

Pedro Alves Jan. 8, 2015, 11:39 a.m. UTC | #1
On 01/08/2015 04:04 AM, Patrick Palka wrote:
> The function key_is_command_char() is simply a predicate that determines
> whether the function tui_dispatch_ctrl_char() will do anything useful.
> Since tui_dispatch_ctrl_char() performs the same checks as
> key_is_command_char() it is unnecessary to keep key_is_command_char()
> around.  This patch removes this useless function and instead
> unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
> tui_getc().
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-io.c (tui_getc): Don't call key_is_command_char.
> 	(key_is_command_char): Delete.

OK.

Thanks,
Pedro Alves
  
Eli Zaretskii Jan. 8, 2015, 1:34 p.m. UTC | #2
> From: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Patrick Palka <patrick@parcs.ath.cx>
> Date: Wed,  7 Jan 2015 23:04:43 -0500
> 
> The function key_is_command_char() is simply a predicate that determines
> whether the function tui_dispatch_ctrl_char() will do anything useful.
> Since tui_dispatch_ctrl_char() performs the same checks as
> key_is_command_char() it is unnecessary to keep key_is_command_char()
> around.  This patch removes this useless function and instead
> unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
> tui_getc().

But tui_dispatch_ctrl_char punts when the current window is the
command window, doesn't it?  This means we are losing the possibility
to handle command keys in the command window.

Thanks.
  
Patrick Palka Jan. 8, 2015, 1:47 p.m. UTC | #3
On Thu, Jan 8, 2015 at 8:34 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Patrick Palka <patrick@parcs.ath.cx>
>> Cc: Patrick Palka <patrick@parcs.ath.cx>
>> Date: Wed,  7 Jan 2015 23:04:43 -0500
>>
>> The function key_is_command_char() is simply a predicate that determines
>> whether the function tui_dispatch_ctrl_char() will do anything useful.
>> Since tui_dispatch_ctrl_char() performs the same checks as
>> key_is_command_char() it is unnecessary to keep key_is_command_char()
>> around.  This patch removes this useless function and instead
>> unconditionally calls tui_dispatch_ctrl_char() inside its only caller,
>> tui_getc().
>
> But tui_dispatch_ctrl_char punts when the current window is the
> command window, doesn't it?  This means we are losing the possibility
> to handle command keys in the command window.

I don't see how that follows.  If the current window is the command
window then dispatch_ctrl_char() returns the input character
unmodified.  So the behavior after the patch is the same as if the
call to dispatch_ctrl_char() was guarded by the key_is_command_char()
predicate, before the patch.

>
> Thanks.
  
Eli Zaretskii Jan. 8, 2015, 2:58 p.m. UTC | #4
> From: Patrick Palka <patrick@parcs.ath.cx>
> Date: Thu, 8 Jan 2015 08:47:44 -0500
> Cc: gdb-patches@sourceware.org
> 
> > But tui_dispatch_ctrl_char punts when the current window is the
> > command window, doesn't it?  This means we are losing the possibility
> > to handle command keys in the command window.
> 
> I don't see how that follows.  If the current window is the command
> window then dispatch_ctrl_char() returns the input character
> unmodified.  So the behavior after the patch is the same as if the
> call to dispatch_ctrl_char() was guarded by the key_is_command_char()
> predicate, before the patch.

Sorry, you are right.
  

Patch

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 233e7a6..b0e6c75 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -63,17 +63,6 @@  key_is_backspace (int ch)
   return (ch == 8);
 }
 
-int
-key_is_command_char (int ch)
-{
-  return ((ch == KEY_NPAGE) || (ch == KEY_PPAGE)
-	  || (ch == KEY_LEFT) || (ch == KEY_RIGHT)
-	  || (ch == KEY_UP) || (ch == KEY_DOWN)
-	  || (ch == KEY_SF) || (ch == KEY_SR)
-	  || (ch == (int)'\f') 
-	  || key_is_start_sequence (ch));
-}
-
 /* Use definition from readline 4.3.  */
 #undef CTRL_CHAR
 #define CTRL_CHAR(c) \
@@ -682,10 +671,8 @@  tui_getc (FILE *fp)
         }
     }
   
-  if (key_is_command_char (ch))
-    {				/* 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 == '\n' || ch == '\r' || ch == '\f')
     TUI_CMD_WIN->detail.command_info.curch = 0;