[20/20] Change TUI window commands to be case-sensitive

Message ID 87mufah4y2.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Sept. 11, 2019, 9:57 p.m. UTC
  >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tom Tromey <tom@tromey.com>
>> Cc: Tom Tromey <tom@tromey.com>
>> Date: Tue, 10 Sep 2019 13:08:57 -0600
>> 
>> The TUI window-related commands like "focus" are case insensitive.
>> This is not the norm in gdb, and I don't see a good reason to have it
>> here.  This patch changes the TUI to be case sensitive, like the rest
>> of gdb.

Eli> Shouldn't this be in NEWS?  It's an incompatible behavior change.

Good idea, here's an updated patch.

Tom

commit db427ce6225a1fc795261f5f5428cf1a7e99be52
Author: Tom Tromey <tom@tromey.com>
Date:   Tue Jul 23 16:01:03 2019 -0600

    Change TUI window commands to be case-sensitive
    
    The TUI window-related commands like "focus" are case insensitive.
    This is not the norm in gdb, and I don't see a good reason to have it
    here.  This patch changes the TUI to be case sensitive, like the rest
    of gdb.
    
    gdb/ChangeLog
    2019-09-11  Tom Tromey  <tom@tromey.com>
    
            * NEWS: Mention case-sensitivity of TUI commands.
            * tui/tui-win.c (tui_set_focus_command): Now case-sensitive.
            (tui_set_win_height_command, parse_scrolling_args): Likewise.
            * tui/tui-layout.c (tui_layout_command): Now case-sensitive.
  

Comments

Eli Zaretskii Sept. 12, 2019, 3:29 a.m. UTC | #1
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Wed, 11 Sep 2019 15:57:09 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Tom Tromey <tom@tromey.com>
> >> Cc: Tom Tromey <tom@tromey.com>
> >> Date: Tue, 10 Sep 2019 13:08:57 -0600
> >> 
> >> The TUI window-related commands like "focus" are case insensitive.
> >> This is not the norm in gdb, and I don't see a good reason to have it
> >> here.  This patch changes the TUI to be case sensitive, like the rest
> >> of gdb.
> 
> Eli> Shouldn't this be in NEWS?  It's an incompatible behavior change.
> 
> Good idea, here's an updated patch.

The NEWS part is OK, thanks.
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 59ae111336f..b5ccdc3b8e2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@ 
+2019-09-11  Tom Tromey  <tom@tromey.com>
+
+	* NEWS: Mention case-sensitivity of TUI commands.
+	* tui/tui-win.c (tui_set_focus_command): Now case-sensitive.
+	(tui_set_win_height_command, parse_scrolling_args): Likewise.
+	* tui/tui-layout.c (tui_layout_command): Now case-sensitive.
+
 2019-09-10  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-source.c (tui_source_window::set_contents): Use
diff --git a/gdb/NEWS b/gdb/NEWS
index f382e887c0c..1b2f616e7fb 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -181,6 +181,9 @@  maint show test-options-completion-result
   Commands used by the testsuite to validate the command options
   framework.
 
+focus, winheight, +, -, >, <
+  These commands are now case-sensitive.
+
 * New command options, command completion
 
   GDB now has a standard infrastructure to support dash-style command
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 0f3e8d945e8..7aa670ec69d 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -337,29 +337,23 @@  Layout names are:\n\
 static void
 tui_layout_command (const char *layout_name, int from_tty)
 {
-  int i;
   enum tui_layout_type new_layout = UNDEFINED_LAYOUT;
   enum tui_layout_type cur_layout = tui_current_layout ();
 
-  if (layout_name == NULL)
+  if (layout_name == NULL || *layout_name == '\0')
     error (_("Usage: layout prev | next | LAYOUT-NAME"));
 
-  std::string copy = layout_name;
-  for (i = 0; i < copy.size (); i++)
-    copy[i] = toupper (copy[i]);
-  const char *buf_ptr = copy.c_str ();
-
   /* First check for ambiguous input.  */
-  if (strlen (buf_ptr) <= 1 && *buf_ptr == 'S')
+  if (strcmp (layout_name, "s") == 0)
     error (_("Ambiguous command input."));
 
-  if (subset_compare (buf_ptr, "SRC"))
+  if (subset_compare (layout_name, "src"))
     new_layout = SRC_COMMAND;
-  else if (subset_compare (buf_ptr, "ASM"))
+  else if (subset_compare (layout_name, "asm"))
     new_layout = DISASSEM_COMMAND;
-  else if (subset_compare (buf_ptr, "SPLIT"))
+  else if (subset_compare (layout_name, "split"))
     new_layout = SRC_DISASSEM_COMMAND;
-  else if (subset_compare (buf_ptr, "REGS"))
+  else if (subset_compare (layout_name, "regs"))
     {
       if (cur_layout == SRC_COMMAND
 	  || cur_layout == SRC_DATA_COMMAND)
@@ -367,9 +361,9 @@  tui_layout_command (const char *layout_name, int from_tty)
       else
 	new_layout = DISASSEM_DATA_COMMAND;
     }
-  else if (subset_compare (buf_ptr, "NEXT"))
+  else if (subset_compare (layout_name, "next"))
     new_layout = next_layout ();
-  else if (subset_compare (buf_ptr, "PREV"))
+  else if (subset_compare (layout_name, "prev"))
     new_layout = prev_layout ();
   else
     error (_("Unrecognized layout: %s"), layout_name);
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index aecb7791f0a..37e22c550f9 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -776,35 +776,27 @@  tui_set_focus_command (const char *arg, int from_tty)
 
   if (arg != NULL)
     {
-      char *buf_ptr = xstrdup (arg);
-      int i;
       struct tui_win_info *win_info = NULL;
 
-      for (i = 0; (i < strlen (buf_ptr)); i++)
-	buf_ptr[i] = tolower (arg[i]);
-
-      if (subset_compare (buf_ptr, "next"))
+      if (subset_compare (arg, "next"))
 	win_info = tui_next_win (tui_win_with_focus ());
-      else if (subset_compare (buf_ptr, "prev"))
+      else if (subset_compare (arg, "prev"))
 	win_info = tui_prev_win (tui_win_with_focus ());
       else
-	win_info = tui_partial_win_by_name (buf_ptr);
+	win_info = tui_partial_win_by_name (arg);
 
-      if (win_info == NULL || !win_info->is_visible ())
-	warning (_("Invalid window specified. \n\
-The window name specified must be valid and visible.\n"));
-      else
-	{
-	  tui_set_win_focus_to (win_info);
-	  keypad (TUI_CMD_WIN->handle, (win_info != TUI_CMD_WIN));
-	}
+      if (win_info == NULL)
+	error (_("Unrecognized window name \"%s\""), arg);
+      if (!win_info->is_visible ())
+	error (_("Window \"%s\" is not visible"), arg);
 
-      xfree (buf_ptr);
+      tui_set_win_focus_to (win_info);
+      keypad (TUI_CMD_WIN->handle, (win_info != TUI_CMD_WIN));
       printf_filtered (_("Focus set to %s window.\n"),
 		       tui_win_with_focus ()->name ());
     }
   else
-    warning (_("Incorrect Number of Arguments.\n%s"), FOCUS_USAGE);
+    error (_("Incorrect Number of Arguments.\n%s"), FOCUS_USAGE);
 }
 
 static void
@@ -927,65 +919,61 @@  tui_set_win_height_command (const char *arg, int from_tty)
       char *buf = &copy[0];
       char *buf_ptr = buf;
       char *wname = NULL;
-      int new_height, i;
+      int new_height;
       struct tui_win_info *win_info;
 
       wname = buf_ptr;
       buf_ptr = strchr (buf_ptr, ' ');
       if (buf_ptr != NULL)
 	{
-	  *buf_ptr = (char) 0;
+	  *buf_ptr = '\0';
 
 	  /* Validate the window name.  */
-	  for (i = 0; i < strlen (wname); i++)
-	    wname[i] = tolower (wname[i]);
 	  win_info = tui_partial_win_by_name (wname);
 
-	  if (win_info == NULL || !win_info->is_visible ())
-	    warning (_("Invalid window specified. \n\
-The window name specified must be valid and visible.\n"));
-	  else
+	  if (win_info == NULL)
+	    error (_("Unrecognized window name \"%s\""), arg);
+	  if (!win_info->is_visible ())
+	    error (_("Window \"%s\" is not visible"), arg);
+
+	  /* Process the size.  */
+	  buf_ptr = skip_spaces (buf_ptr);
+
+	  if (*buf_ptr != '\0')
 	    {
-	      /* Process the size.  */
-	      while (*(++buf_ptr) == ' ')
-		;
+	      bool negate = false;
+	      bool fixed_size = true;
+	      int input_no;;
 
-	      if (*buf_ptr != (char) 0)
+	      if (*buf_ptr == '+' || *buf_ptr == '-')
 		{
-		  int negate = FALSE;
-		  int fixed_size = TRUE;
-		  int input_no;;
-
-		  if (*buf_ptr == '+' || *buf_ptr == '-')
-		    {
-		      if (*buf_ptr == '-')
-			negate = TRUE;
-		      fixed_size = FALSE;
-		      buf_ptr++;
-		    }
-		  input_no = atoi (buf_ptr);
-		  if (input_no > 0)
-		    {
-		      if (negate)
-			input_no *= (-1);
-		      if (fixed_size)
-			new_height = input_no;
-		      else
-			new_height = win_info->height + input_no;
-
-		      /* Now change the window's height, and adjust
-		         all other windows around it.  */
-		      if (tui_adjust_win_heights (win_info,
-						new_height) == TUI_FAILURE)
-			warning (_("Invalid window height specified.\n%s"),
-				 WIN_HEIGHT_USAGE);
-		      else
-                        tui_update_gdb_sizes ();
-		    }
+		  if (*buf_ptr == '-')
+		    negate = true;
+		  fixed_size = false;
+		  buf_ptr++;
+		}
+	      input_no = atoi (buf_ptr);
+	      if (input_no > 0)
+		{
+		  if (negate)
+		    input_no *= (-1);
+		  if (fixed_size)
+		    new_height = input_no;
 		  else
+		    new_height = win_info->height + input_no;
+
+		  /* Now change the window's height, and adjust
+		     all other windows around it.  */
+		  if (tui_adjust_win_heights (win_info,
+					      new_height) == TUI_FAILURE)
 		    warning (_("Invalid window height specified.\n%s"),
 			     WIN_HEIGHT_USAGE);
+		  else
+		    tui_update_gdb_sizes ();
 		}
+	      else
+		warning (_("Invalid window height specified.\n%s"),
+			 WIN_HEIGHT_USAGE);
 	    }
 	}
       else
@@ -1299,7 +1287,7 @@  parse_scrolling_args (const char *arg,
 	  buf_ptr = strchr (buf_ptr, ' ');
 	  if (buf_ptr != NULL)
 	    {
-	      *buf_ptr = (char) 0;
+	      *buf_ptr = '\0';
 	      if (num_to_scroll)
 		*num_to_scroll = atoi (num_str);
 	      buf_ptr++;
@@ -1313,29 +1301,19 @@  parse_scrolling_args (const char *arg,
 	{
 	  const char *wname;
 
-	  if (*buf_ptr == ' ')
-	    while (*(++buf_ptr) == ' ')
-	      ;
+	  wname = skip_spaces (buf_ptr);
 
-	  if (*buf_ptr != (char) 0)
+	  if (*wname != '\0')
 	    {
-	      /* Validate the window name.  */
-	      for (char *p = buf_ptr; *p != '\0'; p++)
-		*p = tolower (*p);
-
-	      wname = buf_ptr;
+	      *win_to_scroll = tui_partial_win_by_name (wname);
+
+	      if (*win_to_scroll == NULL)
+		error (_("Unrecognized window `%s'"), wname);
+	      if (!(*win_to_scroll)->is_visible ())
+		error (_("Window is not visible"));
+	      else if (*win_to_scroll == TUI_CMD_WIN)
+		*win_to_scroll = *(tui_source_windows ().begin ());
 	    }
-	  else
-	    wname = "?";
-	  
-	  *win_to_scroll = tui_partial_win_by_name (wname);
-
-	  if (*win_to_scroll == NULL)
-	    error (_("Unrecognized window `%s'"), wname);
-	  if (!(*win_to_scroll)->is_visible ())
-	    error (_("Window is not visible"));
-	  else if (*win_to_scroll == TUI_CMD_WIN)
-	    *win_to_scroll = *(tui_source_windows ().begin ());
 	}
     }
 }