[PATCH] RFC: gdb: redo whitespace-stripping from commands

[PATCH] RFC: gdb: redo whitespace-stripping from commands

[Kartik K. Agaram]

Before this patch, trailing whitespace was not stripped:
  - from 'set'/'show' commands
  - from 'complete' commands

Now I've added the 'with' commands to that list because it contains a
'complete' subcommand. To accomplish this, I'm trying to fix a TODO to
provide a per-command flag controlling whitespace-stripping.

I've also cleaned up some seemingly TODOs that are either fixed by this
patch or obsolete.

Open question: This patch stops stripping trailing whitespace from all
'with' commands. Might that create issues with other 'with' commands
besides 'with ... -- complete'?

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29784
---
 gdb/cli/cli-cmds.c   | 12 +++++-------
 gdb/cli/cli-cmds.h   |  4 ----
 gdb/cli/cli-decode.h |  8 ++++----
 gdb/top.c            | 12 +-----------
 4 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 9a5021f9d08..2cb7471dda8 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -427,12 +427,6 @@ complete_command (const char *arg, int from_tty)
     }
 }
 
-int
-is_complete_command (struct cmd_list_element *c)
-{
-  return cmd_simple_func_eq (c, complete_command);
-}
-
 static void
 show_version (const char *args, int from_tty)
 {
@@ -2702,14 +2696,17 @@ Generic command for showing things about the program being debugged."),
   add_com_alias ("i", info_cmd, class_info, 1);
   add_com_alias ("inf", info_cmd, class_info, 1);
 
-  add_com ("complete", class_obscure, complete_command,
+  cmd_list_element *complete_cmd
+    = add_com ("complete", class_obscure, complete_command,
 	   _("List the completions for the rest of the line as a command."));
+  complete_cmd->strip_trailing_white_space_p = 0;
 
   c = add_show_prefix_cmd ("show", class_info, _("\
 Generic command for showing things about the debugger."),
 			   &showlist, 0, &cmdlist);
   /* Another way to get at the same thing.  */
   add_alias_cmd ("set", c, class_info, 0, &infolist);
+  c->strip_trailing_white_space_p = 0;
 
   cmd_list_element *with_cmd
     = add_com ("with", class_vars, with_command, _("\
@@ -2726,6 +2723,7 @@ E.g.:\n\
 You can change multiple settings using nested with, and use\n\
 abbreviations for commands and/or values.  E.g.:\n\
   w la p -- w p el u -- p obj"));
+  with_cmd->strip_trailing_white_space_p = 0;
   set_cmd_completer_handle_brkchars (with_cmd, with_command_completer);
   add_com_alias ("w", with_cmd, class_vars, 1);
 
diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
index 33d13fb8563..c3004a8e857 100644
--- a/gdb/cli/cli-cmds.h
+++ b/gdb/cli/cli-cmds.h
@@ -149,10 +149,6 @@ extern struct cmd_list_element *showsourcelist;
 
 extern unsigned int max_user_call_depth;
 
-/* Exported to gdb/top.c */
-
-int is_complete_command (struct cmd_list_element *cmd);
-
 /* Exported to gdb/main.c */
 
 extern void cd_command (const char *, int);
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 9be446fb641..f794e3c45f6 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -60,6 +60,7 @@ struct cmd_list_element
       allow_unknown (0),
       abbrev_flag (0),
       type (not_set_cmd),
+      strip_trailing_white_space_p (1),
       doc (doc_)
   {
     gdb_assert (name != nullptr);
@@ -175,11 +176,10 @@ struct cmd_list_element
      or "show").  */
   ENUM_BITFIELD (cmd_types) type : 2;
 
+  unsigned int strip_trailing_white_space_p : 1;
+
   /* Function definition of this command.  NULL for command class
-     names and for help topics that are not really commands.  NOTE:
-     cagney/2002-02-02: This function signature is evolving.  For
-     the moment suggest sticking with either set_cmd_cfunc() or
-     set_cmd_sfunc().  */
+     names and for help topics that are not really commands.  */
   cmd_func_ftype *func;
 
   /* The command's real callback.  At present func() bounces through
diff --git a/gdb/top.c b/gdb/top.c
index 6adef467b90..8a1e586b4c1 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -503,20 +503,10 @@ execute_command (const char *p, int from_tty)
 	  arg = *p == '\0' ? nullptr : p;
 	}
 
-      /* FIXME: cagney/2002-02-02: The c->type test is pretty dodgy
-	 while the is_complete_command(cfunc) test is just plain
-	 bogus.  They should both be replaced by a test of the form
-	 c->strip_trailing_white_space_p.  */
-      /* NOTE: cagney/2002-02-02: The function.cfunc in the below
-	 can't be replaced with func.  This is because it is the
-	 cfunc, and not the func, that has the value that the
-	 is_complete_command hack is testing for.  */
       /* Clear off trailing whitespace, except for set and complete
 	 command.  */
       std::string without_whitespace;
-      if (arg
-	  && c->type != set_cmd
-	  && !is_complete_command (c))
+      if (arg && c->strip_trailing_white_space_p)
 	{
 	  const char *old_end = arg + strlen (arg) - 1;
 	  p = old_end;
-- 
2.49.0

Re: [PATCH] RFC: gdb: redo whitespace-stripping from commands

[Tom Tromey]

>>>>> "Kartik" == Kartik K Agaram <ak@akkartik.com> writes:

Thanks for the patch.

Kartik> Before this patch, trailing whitespace was not stripped:
Kartik>   - from 'set'/'show' commands
Kartik>   - from 'complete' commands

Kartik> Now I've added the 'with' commands to that list because it contains a
Kartik> 'complete' subcommand. To accomplish this, I'm trying to fix a TODO to
Kartik> provide a per-command flag controlling whitespace-stripping.

Kartik> I've also cleaned up some seemingly TODOs that are either fixed by this
Kartik> patch or obsolete.

Kartik> Open question: This patch stops stripping trailing whitespace from all
Kartik> 'with' commands. Might that create issues with other 'with' commands
Kartik> besides 'with ... -- complete'?

Maybe it would cause problems with trailing whitespace showing up in
filenames?  Or with commands that don't expect an argument.  For those
commands, completion might add a trailing space, causing the function to
be called with " " rather than nullptr.

I wonder if the 'with' command itself -- and any other commands that
invoke commands, like 'eval' -- should strip whitespace as appropriate
before invocation.  That is, move the whitespace stripping code
somewhere else.  (Assuming it's in the wrong spot, I don't recall off
the top of my head where this stuff happens.)

Kartik> +  complete_cmd->strip_trailing_white_space_p = 0;
 
Kartik> +  c->strip_trailing_white_space_p = 0;
 
Kartik> +      strip_trailing_white_space_p (1),

Kartik> +  unsigned int strip_trailing_white_space_p : 1;

This should be bool and the constants above should be true/false.

Tom

Re: [PATCH] RFC: gdb: redo whitespace-stripping from commands

[Kartik Agaram]

Thanks for all your comments.

On Thu, May 29, 2025, at 06:36, Tom Tromey wrote:
> This should be bool and the constants above should be true/false.

You're right, my knowledge of C is decades out of date. Fixed locally. Let me think about the best place to strip whitespace in subcommands and I'll submit a new version after that.

Re: [PATCH] RFC: gdb: redo whitespace-stripping from commands

[Guinevere Larsen]

On 5/29/25 10:36 AM, Tom Tromey wrote:
>>>>>> "Kartik" == Kartik K Agaram <ak@akkartik.com> writes:
> Thanks for the patch.
>
> Kartik> Before this patch, trailing whitespace was not stripped:
> Kartik>   - from 'set'/'show' commands
> Kartik>   - from 'complete' commands
>
> Kartik> Now I've added the 'with' commands to that list because it contains a
> Kartik> 'complete' subcommand. To accomplish this, I'm trying to fix a TODO to
> Kartik> provide a per-command flag controlling whitespace-stripping.
>
> Kartik> I've also cleaned up some seemingly TODOs that are either fixed by this
> Kartik> patch or obsolete.
>
> Kartik> Open question: This patch stops stripping trailing whitespace from all
> Kartik> 'with' commands. Might that create issues with other 'with' commands
> Kartik> besides 'with ... -- complete'?
>
> Maybe it would cause problems with trailing whitespace showing up in
> filenames?  Or with commands that don't expect an argument.  For those
> commands, completion might add a trailing space, causing the function to
> be called with " " rather than nullptr.
>
> I wonder if the 'with' command itself -- and any other commands that
> invoke commands, like 'eval' -- should strip whitespace as appropriate
> before invocation.  That is, move the whitespace stripping code
> somewhere else.  (Assuming it's in the wrong spot, I don't recall off
> the top of my head where this stuff happens.)

Removing trailing whitespace is done in the execute_command function in 
top.c. The command "with" will already call execute_command itself to 
invoke the correct command, so I think the correct way to go about it is 
for those commands to never strip whitespace, and then always call 
execute_command, which will do the right thing based on the inner command.

>
> Kartik> +  complete_cmd->strip_trailing_white_space_p = 0;
>   
> Kartik> +  c->strip_trailing_white_space_p = 0;
>   
> Kartik> +      strip_trailing_white_space_p (1),
>
> Kartik> +  unsigned int strip_trailing_white_space_p : 1;
>
> This should be bool and the constants above should be true/false.
Should the name end in _p? I would think _p is for pointers, and this is 
just a bool, but maybe I'm mistaken.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Tom
>

Re: [PATCH] RFC: gdb: redo whitespace-stripping from commands

[Kartik Agaram]

On Thu, May 29, 2025, at 06:53, Guinevere Larsen wrote:
> Should the name end in _p? I would think _p is for pointers, and this is 
> just a bool, but maybe I'm mistaken.

I certainly don't know what I'm doing here. I was following cagney's idea in a TODO, but that's from 2002.

I'm looking around now, and at least in the cli/ subdirectory I do see this Lisp idiom of predicates getting a _p suffix. But happy to go with whatever y'all suggest.

Thanks for the hint on subcommands, I'll try it out.

Re: [PATCH] RFC: gdb: redo whitespace-stripping from commands

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> Should the name end in _p? I would think _p is for pointers, and this
Guinevere> is just a bool, but maybe I'm mistaken.

"_p" is a convention from the lisp world, search for "-p" here:
https://www.gnu.org/software/emacs/manual/html_node/elisp/Coding-Conventions.html

I've always assumed it dated back to when RMS first wrote gdb, but I
don't really know.

Anyway nowadays we don't use it as much, IIRC Simon dislikes it.

Tom

gdb: redo whitespace-stripping from commands

[Kartik K. Agaram]

All tests pass now.

> Open question: This patch stops stripping trailing whitespace from all
> 'with' commands. Might that create issues with other 'with' commands besides
> 'with ... -- complete'?

I've answered this question to my satisfaction. 'with' recursively calls
execute_command, so other 'with' commands besides 'set' or 'complete' will
result in the following timeline of events:

  execute_command()
    - command is 'with' so don't strip whitespace
    - execute_command()
      - command is not 'with' or 'set' or 'complete' so strip whitespace
      - ...

[PATCH] gdb: redo whitespace-stripping from commands

[Kartik K. Agaram]

Before this patch, trailing whitespace was not stripped from 'set' and
'complete' commands. Now I've added 'with' commands to that list because
they contain a 'complete' subcommand. To accomplish this, I'm fixing an
ancient TODO to provide a per-command flag controlling
whitespace-stripping.

Any subcommands of 'with' besides 'complete' or 'set' will continue to
work because they go through a recursive call to execute_command. The
outer call for 'with' won't strip whitespace, then the recursive call
will.

I've also cleaned up 2 obsolete TODOs and NOTEs in related code.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29784
---
 gdb/cli/cli-cmds.c   | 11 ++++-------
 gdb/cli/cli-cmds.h   |  4 ----
 gdb/cli/cli-decode.c |  1 +
 gdb/cli/cli-decode.h |  8 ++++----
 gdb/top.c            | 12 +-----------
 5 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 9a5021f9d08..f2d6d576cb8 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -427,12 +427,6 @@ complete_command (const char *arg, int from_tty)
     }
 }
 
-int
-is_complete_command (struct cmd_list_element *c)
-{
-  return cmd_simple_func_eq (c, complete_command);
-}
-
 static void
 show_version (const char *args, int from_tty)
 {
@@ -2702,8 +2696,10 @@ Generic command for showing things about the program being debugged."),
   add_com_alias ("i", info_cmd, class_info, 1);
   add_com_alias ("inf", info_cmd, class_info, 1);
 
-  add_com ("complete", class_obscure, complete_command,
+  cmd_list_element *complete_cmd
+    = add_com ("complete", class_obscure, complete_command,
 	   _("List the completions for the rest of the line as a command."));
+  complete_cmd->strip_trailing_white_space_p = false;
 
   c = add_show_prefix_cmd ("show", class_info, _("\
 Generic command for showing things about the debugger."),
@@ -2726,6 +2722,7 @@ E.g.:\n\
 You can change multiple settings using nested with, and use\n\
 abbreviations for commands and/or values.  E.g.:\n\
   w la p -- w p el u -- p obj"));
+  with_cmd->strip_trailing_white_space_p = false;
   set_cmd_completer_handle_brkchars (with_cmd, with_command_completer);
   add_com_alias ("w", with_cmd, class_vars, 1);
 
diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
index 33d13fb8563..c3004a8e857 100644
--- a/gdb/cli/cli-cmds.h
+++ b/gdb/cli/cli-cmds.h
@@ -149,10 +149,6 @@ extern struct cmd_list_element *showsourcelist;
 
 extern unsigned int max_user_call_depth;
 
-/* Exported to gdb/top.c */
-
-int is_complete_command (struct cmd_list_element *cmd);
-
 /* Exported to gdb/main.c */
 
 extern void cd_command (const char *, int);
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 48a34667c37..782d38f781f 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -582,6 +582,7 @@ add_setshow_cmd_full_erased (const char *name,
 			     extra_literals, args,
 			     full_set_doc.release (), set_list);
   set->doc_allocated = 1;
+  set->strip_trailing_white_space_p = false;
 
   if (set_func != NULL)
     set->func = set_func;
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 9be446fb641..d3d512adcd4 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -60,6 +60,7 @@ struct cmd_list_element
       allow_unknown (0),
       abbrev_flag (0),
       type (not_set_cmd),
+      strip_trailing_white_space_p (true),
       doc (doc_)
   {
     gdb_assert (name != nullptr);
@@ -175,11 +176,10 @@ struct cmd_list_element
      or "show").  */
   ENUM_BITFIELD (cmd_types) type : 2;
 
+  bool strip_trailing_white_space_p;
+
   /* Function definition of this command.  NULL for command class
-     names and for help topics that are not really commands.  NOTE:
-     cagney/2002-02-02: This function signature is evolving.  For
-     the moment suggest sticking with either set_cmd_cfunc() or
-     set_cmd_sfunc().  */
+     names and for help topics that are not really commands.  */
   cmd_func_ftype *func;
 
   /* The command's real callback.  At present func() bounces through
diff --git a/gdb/top.c b/gdb/top.c
index 6adef467b90..8a1e586b4c1 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -503,20 +503,10 @@ execute_command (const char *p, int from_tty)
 	  arg = *p == '\0' ? nullptr : p;
 	}
 
-      /* FIXME: cagney/2002-02-02: The c->type test is pretty dodgy
-	 while the is_complete_command(cfunc) test is just plain
-	 bogus.  They should both be replaced by a test of the form
-	 c->strip_trailing_white_space_p.  */
-      /* NOTE: cagney/2002-02-02: The function.cfunc in the below
-	 can't be replaced with func.  This is because it is the
-	 cfunc, and not the func, that has the value that the
-	 is_complete_command hack is testing for.  */
       /* Clear off trailing whitespace, except for set and complete
 	 command.  */
       std::string without_whitespace;
-      if (arg
-	  && c->type != set_cmd
-	  && !is_complete_command (c))
+      if (arg && c->strip_trailing_white_space_p)
 	{
 	  const char *old_end = arg + strlen (arg) - 1;
 	  p = old_end;
-- 
2.49.0

Re: [PATCH] gdb: redo whitespace-stripping from commands

[Guinevere Larsen]

Hi Kartik!

I have one minor wording suggestion in the commit message, but otherwise 
I think this change is great!
On 5/31/25 10:17 PM, Kartik K. Agaram wrote:
> Before this patch, trailing whitespace was not stripped from 'set' and
> 'complete' commands. Now I've added 'with' commands to that list because
> they contain a 'complete' subcommand. To accomplish this, I'm fixing an

I'd word it as "I've added 'with' to that list of commands because it 
may contain one of those commands".

Feel free to add my review trailer to the end of your commit message, 
and I hope this gets approved by a global maintainer soon!

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> ancient TODO to provide a per-command flag controlling
> whitespace-stripping.
>
> Any subcommands of 'with' besides 'complete' or 'set' will continue to
> work because they go through a recursive call to execute_command. The
> outer call for 'with' won't strip whitespace, then the recursive call
> will.
>
> I've also cleaned up 2 obsolete TODOs and NOTEs in related code.
>
> Tested on x86_64-linux.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29784
> ---
>   gdb/cli/cli-cmds.c   | 11 ++++-------
>   gdb/cli/cli-cmds.h   |  4 ----
>   gdb/cli/cli-decode.c |  1 +
>   gdb/cli/cli-decode.h |  8 ++++----
>   gdb/top.c            | 12 +-----------
>   5 files changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 9a5021f9d08..f2d6d576cb8 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -427,12 +427,6 @@ complete_command (const char *arg, int from_tty)
>       }
>   }
>   
> -int
> -is_complete_command (struct cmd_list_element *c)
> -{
> -  return cmd_simple_func_eq (c, complete_command);
> -}
> -
>   static void
>   show_version (const char *args, int from_tty)
>   {
> @@ -2702,8 +2696,10 @@ Generic command for showing things about the program being debugged."),
>     add_com_alias ("i", info_cmd, class_info, 1);
>     add_com_alias ("inf", info_cmd, class_info, 1);
>   
> -  add_com ("complete", class_obscure, complete_command,
> +  cmd_list_element *complete_cmd
> +    = add_com ("complete", class_obscure, complete_command,
>   	   _("List the completions for the rest of the line as a command."));
> +  complete_cmd->strip_trailing_white_space_p = false;
>   
>     c = add_show_prefix_cmd ("show", class_info, _("\
>   Generic command for showing things about the debugger."),
> @@ -2726,6 +2722,7 @@ E.g.:\n\
>   You can change multiple settings using nested with, and use\n\
>   abbreviations for commands and/or values.  E.g.:\n\
>     w la p -- w p el u -- p obj"));
> +  with_cmd->strip_trailing_white_space_p = false;
>     set_cmd_completer_handle_brkchars (with_cmd, with_command_completer);
>     add_com_alias ("w", with_cmd, class_vars, 1);
>   
> diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
> index 33d13fb8563..c3004a8e857 100644
> --- a/gdb/cli/cli-cmds.h
> +++ b/gdb/cli/cli-cmds.h
> @@ -149,10 +149,6 @@ extern struct cmd_list_element *showsourcelist;
>   
>   extern unsigned int max_user_call_depth;
>   
> -/* Exported to gdb/top.c */
> -
> -int is_complete_command (struct cmd_list_element *cmd);
> -
>   /* Exported to gdb/main.c */
>   
>   extern void cd_command (const char *, int);
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index 48a34667c37..782d38f781f 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -582,6 +582,7 @@ add_setshow_cmd_full_erased (const char *name,
>   			     extra_literals, args,
>   			     full_set_doc.release (), set_list);
>     set->doc_allocated = 1;
> +  set->strip_trailing_white_space_p = false;
>   
>     if (set_func != NULL)
>       set->func = set_func;
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index 9be446fb641..d3d512adcd4 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -60,6 +60,7 @@ struct cmd_list_element
>         allow_unknown (0),
>         abbrev_flag (0),
>         type (not_set_cmd),
> +      strip_trailing_white_space_p (true),
>         doc (doc_)
>     {
>       gdb_assert (name != nullptr);
> @@ -175,11 +176,10 @@ struct cmd_list_element
>        or "show").  */
>     ENUM_BITFIELD (cmd_types) type : 2;
>   
> +  bool strip_trailing_white_space_p;
> +
>     /* Function definition of this command.  NULL for command class
> -     names and for help topics that are not really commands.  NOTE:
> -     cagney/2002-02-02: This function signature is evolving.  For
> -     the moment suggest sticking with either set_cmd_cfunc() or
> -     set_cmd_sfunc().  */
> +     names and for help topics that are not really commands.  */
>     cmd_func_ftype *func;
>   
>     /* The command's real callback.  At present func() bounces through
> diff --git a/gdb/top.c b/gdb/top.c
> index 6adef467b90..8a1e586b4c1 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -503,20 +503,10 @@ execute_command (const char *p, int from_tty)
>   	  arg = *p == '\0' ? nullptr : p;
>   	}
>   
> -      /* FIXME: cagney/2002-02-02: The c->type test is pretty dodgy
> -	 while the is_complete_command(cfunc) test is just plain
> -	 bogus.  They should both be replaced by a test of the form
> -	 c->strip_trailing_white_space_p.  */
> -      /* NOTE: cagney/2002-02-02: The function.cfunc in the below
> -	 can't be replaced with func.  This is because it is the
> -	 cfunc, and not the func, that has the value that the
> -	 is_complete_command hack is testing for.  */
>         /* Clear off trailing whitespace, except for set and complete
>   	 command.  */
>         std::string without_whitespace;
> -      if (arg
> -	  && c->type != set_cmd
> -	  && !is_complete_command (c))
> +      if (arg && c->strip_trailing_white_space_p)
>   	{
>   	  const char *old_end = arg + strlen (arg) - 1;
>   	  p = old_end;

Re: [PATCH] gdb: redo whitespace-stripping from commands

[Kartik Agaram]

> I'd word it as "I've added 'with' to that list of commands because it 
> may contain one of those commands".

Great suggestion, thank you!

[PATCH v3] gdb: redo whitespace-stripping from commands

[Kartik K. Agaram]

Before this patch, trailing whitespace was not stripped from 'set' and
'complete' commands. Now I've added 'with' to that list of commands
because it may contain one of those commands". To accomplish this, I'm
fixing an ancient TODO to provide a per-command flag controlling
whitespace-stripping.

Any subcommands of 'with' besides 'complete' or 'set' will continue to
strip whitespace as before, because they go through a recursive call to
execute_command. The outer call for 'with' won't strip whitespace, then
the recursive call will.

I've also cleaned up 2 obsolete TODOs and NOTEs in related code.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29784
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
---
Changes for v3:
* improved the commit description
* rebased up to commit 3e3013968bc
* reran all tests
---
 gdb/cli/cli-cmds.c   | 11 ++++-------
 gdb/cli/cli-cmds.h   |  4 ----
 gdb/cli/cli-decode.c |  1 +
 gdb/cli/cli-decode.h |  8 ++++----
 gdb/top.c            | 12 +-----------
 5 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index d812c493937..27e97d15938 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -427,12 +427,6 @@ complete_command (const char *arg, int from_tty)
     }
 }
 
-int
-is_complete_command (struct cmd_list_element *c)
-{
-  return cmd_simple_func_eq (c, complete_command);
-}
-
 static void
 show_version (const char *args, int from_tty)
 {
@@ -2717,8 +2711,10 @@ Generic command for showing things about the program being debugged."),
   add_com_alias ("i", info_cmd, class_info, 1);
   add_com_alias ("inf", info_cmd, class_info, 1);
 
-  add_com ("complete", class_obscure, complete_command,
+  cmd_list_element *complete_cmd
+    = add_com ("complete", class_obscure, complete_command,
 	   _("List the completions for the rest of the line as a command."));
+  complete_cmd->strip_trailing_white_space_p = false;
 
   c = add_show_prefix_cmd ("show", class_info, _("\
 Generic command for showing things about the debugger."),
@@ -2741,6 +2737,7 @@ E.g.:\n\
 You can change multiple settings using nested with, and use\n\
 abbreviations for commands and/or values.  E.g.:\n\
   w la p -- w p el u -- p obj"));
+  with_cmd->strip_trailing_white_space_p = false;
   set_cmd_completer_handle_brkchars (with_cmd, with_command_completer);
   add_com_alias ("w", with_cmd, class_vars, 1);
 
diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
index 33d13fb8563..c3004a8e857 100644
--- a/gdb/cli/cli-cmds.h
+++ b/gdb/cli/cli-cmds.h
@@ -149,10 +149,6 @@ extern struct cmd_list_element *showsourcelist;
 
 extern unsigned int max_user_call_depth;
 
-/* Exported to gdb/top.c */
-
-int is_complete_command (struct cmd_list_element *cmd);
-
 /* Exported to gdb/main.c */
 
 extern void cd_command (const char *, int);
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 48a34667c37..782d38f781f 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -582,6 +582,7 @@ add_setshow_cmd_full_erased (const char *name,
 			     extra_literals, args,
 			     full_set_doc.release (), set_list);
   set->doc_allocated = 1;
+  set->strip_trailing_white_space_p = false;
 
   if (set_func != NULL)
     set->func = set_func;
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 9be446fb641..d3d512adcd4 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -60,6 +60,7 @@ struct cmd_list_element
       allow_unknown (0),
       abbrev_flag (0),
       type (not_set_cmd),
+      strip_trailing_white_space_p (true),
       doc (doc_)
   {
     gdb_assert (name != nullptr);
@@ -175,11 +176,10 @@ struct cmd_list_element
      or "show").  */
   ENUM_BITFIELD (cmd_types) type : 2;
 
+  bool strip_trailing_white_space_p;
+
   /* Function definition of this command.  NULL for command class
-     names and for help topics that are not really commands.  NOTE:
-     cagney/2002-02-02: This function signature is evolving.  For
-     the moment suggest sticking with either set_cmd_cfunc() or
-     set_cmd_sfunc().  */
+     names and for help topics that are not really commands.  */
   cmd_func_ftype *func;
 
   /* The command's real callback.  At present func() bounces through
diff --git a/gdb/top.c b/gdb/top.c
index 25a2afe09c8..eacccd676f2 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -503,20 +503,10 @@ execute_command (const char *p, int from_tty)
 	  arg = *p == '\0' ? nullptr : p;
 	}
 
-      /* FIXME: cagney/2002-02-02: The c->type test is pretty dodgy
-	 while the is_complete_command(cfunc) test is just plain
-	 bogus.  They should both be replaced by a test of the form
-	 c->strip_trailing_white_space_p.  */
-      /* NOTE: cagney/2002-02-02: The function.cfunc in the below
-	 can't be replaced with func.  This is because it is the
-	 cfunc, and not the func, that has the value that the
-	 is_complete_command hack is testing for.  */
       /* Clear off trailing whitespace, except for set and complete
 	 command.  */
       std::string without_whitespace;
-      if (arg
-	  && c->type != set_cmd
-	  && !is_complete_command (c))
+      if (arg && c->strip_trailing_white_space_p)
 	{
 	  const char *old_end = arg + strlen (arg) - 1;
 	  p = old_end;
-- 
2.49.0

[PATCH v4] gdb: redo whitespace-stripping from commands

[Kartik K. Agaram]

Before this patch, trailing whitespace was not stripped from 'set' and
'complete' commands. Now I've added 'with' to that list of commands
because it may contain one of those commands. To accomplish this, I'm
fixing an ancient TODO to provide a per-command flag controlling
whitespace-stripping.

Any subcommands of 'with' besides 'complete' or 'set' will continue to
strip whitespace as before, because they go through a recursive call to
execute_command. The outer call for 'with' won't strip whitespace, then
the recursive call will.

I've also cleaned up 2 obsolete TODOs and NOTEs in related code.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29784
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
---
Changes for v4:
* Ugh, removed a stray quote character from the commit description.

 gdb/cli/cli-cmds.c   | 11 ++++-------
 gdb/cli/cli-cmds.h   |  4 ----
 gdb/cli/cli-decode.c |  1 +
 gdb/cli/cli-decode.h |  8 ++++----
 gdb/top.c            | 12 +-----------
 5 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index d812c493937..27e97d15938 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -427,12 +427,6 @@ complete_command (const char *arg, int from_tty)
     }
 }
 
-int
-is_complete_command (struct cmd_list_element *c)
-{
-  return cmd_simple_func_eq (c, complete_command);
-}
-
 static void
 show_version (const char *args, int from_tty)
 {
@@ -2717,8 +2711,10 @@ Generic command for showing things about the program being debugged."),
   add_com_alias ("i", info_cmd, class_info, 1);
   add_com_alias ("inf", info_cmd, class_info, 1);
 
-  add_com ("complete", class_obscure, complete_command,
+  cmd_list_element *complete_cmd
+    = add_com ("complete", class_obscure, complete_command,
 	   _("List the completions for the rest of the line as a command."));
+  complete_cmd->strip_trailing_white_space_p = false;
 
   c = add_show_prefix_cmd ("show", class_info, _("\
 Generic command for showing things about the debugger."),
@@ -2741,6 +2737,7 @@ E.g.:\n\
 You can change multiple settings using nested with, and use\n\
 abbreviations for commands and/or values.  E.g.:\n\
   w la p -- w p el u -- p obj"));
+  with_cmd->strip_trailing_white_space_p = false;
   set_cmd_completer_handle_brkchars (with_cmd, with_command_completer);
   add_com_alias ("w", with_cmd, class_vars, 1);
 
diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
index 33d13fb8563..c3004a8e857 100644
--- a/gdb/cli/cli-cmds.h
+++ b/gdb/cli/cli-cmds.h
@@ -149,10 +149,6 @@ extern struct cmd_list_element *showsourcelist;
 
 extern unsigned int max_user_call_depth;
 
-/* Exported to gdb/top.c */
-
-int is_complete_command (struct cmd_list_element *cmd);
-
 /* Exported to gdb/main.c */
 
 extern void cd_command (const char *, int);
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 48a34667c37..782d38f781f 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -582,6 +582,7 @@ add_setshow_cmd_full_erased (const char *name,
 			     extra_literals, args,
 			     full_set_doc.release (), set_list);
   set->doc_allocated = 1;
+  set->strip_trailing_white_space_p = false;
 
   if (set_func != NULL)
     set->func = set_func;
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 9be446fb641..d3d512adcd4 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -60,6 +60,7 @@ struct cmd_list_element
       allow_unknown (0),
       abbrev_flag (0),
       type (not_set_cmd),
+      strip_trailing_white_space_p (true),
       doc (doc_)
   {
     gdb_assert (name != nullptr);
@@ -175,11 +176,10 @@ struct cmd_list_element
      or "show").  */
   ENUM_BITFIELD (cmd_types) type : 2;
 
+  bool strip_trailing_white_space_p;
+
   /* Function definition of this command.  NULL for command class
-     names and for help topics that are not really commands.  NOTE:
-     cagney/2002-02-02: This function signature is evolving.  For
-     the moment suggest sticking with either set_cmd_cfunc() or
-     set_cmd_sfunc().  */
+     names and for help topics that are not really commands.  */
   cmd_func_ftype *func;
 
   /* The command's real callback.  At present func() bounces through
diff --git a/gdb/top.c b/gdb/top.c
index 25a2afe09c8..eacccd676f2 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -503,20 +503,10 @@ execute_command (const char *p, int from_tty)
 	  arg = *p == '\0' ? nullptr : p;
 	}
 
-      /* FIXME: cagney/2002-02-02: The c->type test is pretty dodgy
-	 while the is_complete_command(cfunc) test is just plain
-	 bogus.  They should both be replaced by a test of the form
-	 c->strip_trailing_white_space_p.  */
-      /* NOTE: cagney/2002-02-02: The function.cfunc in the below
-	 can't be replaced with func.  This is because it is the
-	 cfunc, and not the func, that has the value that the
-	 is_complete_command hack is testing for.  */
       /* Clear off trailing whitespace, except for set and complete
 	 command.  */
       std::string without_whitespace;
-      if (arg
-	  && c->type != set_cmd
-	  && !is_complete_command (c))
+      if (arg && c->strip_trailing_white_space_p)
 	{
 	  const char *old_end = arg + strlen (arg) - 1;
 	  p = old_end;
-- 
2.49.0

Re: [PATCH v4] gdb: redo whitespace-stripping from commands

[Kartik Agaram]

Hi, just a friendly ping on this patch. Please let me know if there's anything more I can do for it.

On Fri, Jun 6, 2025, at 09:10, Kartik K. Agaram wrote:
> Before this patch, trailing whitespace was not stripped from 'set' and
> 'complete' commands. Now I've added 'with' to that list of commands
> because it may contain one of those commands. To accomplish this, I'm
> fixing an ancient TODO to provide a per-command flag controlling
> whitespace-stripping.
>
> Any subcommands of 'with' besides 'complete' or 'set' will continue to
> strip whitespace as before, because they go through a recursive call to
> execute_command. The outer call for 'with' won't strip whitespace, then
> the recursive call will.
>
> I've also cleaned up 2 obsolete TODOs and NOTEs in related code.
>
> Tested on x86_64-linux.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29784
> Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
> ---
> Changes for v4:
> * Ugh, removed a stray quote character from the commit description.

Re: [PATCH v4] gdb: redo whitespace-stripping from commands

[Andrew Burgess]

"Kartik K. Agaram" <ak@akkartik.com> writes:

> Before this patch, trailing whitespace was not stripped from 'set' and
> 'complete' commands. Now I've added 'with' to that list of commands
> because it may contain one of those commands. To accomplish this, I'm
> fixing an ancient TODO to provide a per-command flag controlling
> whitespace-stripping.
>
> Any subcommands of 'with' besides 'complete' or 'set' will continue to
> strip whitespace as before, because they go through a recursive call to
> execute_command. The outer call for 'with' won't strip whitespace, then
> the recursive call will.
>
> I've also cleaned up 2 obsolete TODOs and NOTEs in related code.
>
> Tested on x86_64-linux.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29784
> Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
> ---
> Changes for v4:
> * Ugh, removed a stray quote character from the commit description.
>
>  gdb/cli/cli-cmds.c   | 11 ++++-------
>  gdb/cli/cli-cmds.h   |  4 ----
>  gdb/cli/cli-decode.c |  1 +
>  gdb/cli/cli-decode.h |  8 ++++----
>  gdb/top.c            | 12 +-----------
>  5 files changed, 10 insertions(+), 26 deletions(-)

It feels like this commit could have some tests added.  There is a
support library 'completion-support.exp', if you look for the pattern
'load_lib completion-support.exp' in the testsuite then you'll find
plenty of examples of its use.

Do take care though, there are some completion tests that pre-date the
completion-support.exp library -- if you find a test that is manually
sending tabs to GDB without using the support library, then it's
probably an older test, and not a good example to learn from.

Unfortunately, completion tests are towards the harder end of tests to
write (I find anyway), but I think it is worth doing.  Do feel free to
ask if you get stuck.

Thanks,
Andrew