Re: [PATCH v5 3/8] GDB: Add `NUMBER' completion to `set' integer commands

[Bruno Larsen <blarsen@redhat.com>]

On 3/30/22 07:24, Maciej W. Rozycki wrote:
> Fix a completion consistency issue with `set' commands accepting integer
> values and the special `unlimited' keyword:
> 
> (gdb) complete print -elements
> print -elements NUMBER
> print -elements unlimited
> (gdb)
> 
> vs:
> 
> (gdb) complete set print elements
> set print elements unlimited
> (gdb)
> 
> (there is a space entered at the end of both commands, not shown here)
> which also means if you strike <Tab> with `set print elements ' input,
> it will, annoyingly, complete to `set print elements unlimited' right
> away rather than showing a choice between `NUMBER' and `unlimited'.
> 
> Add `NUMBER' then as an available completion for such `set' commands:
> 
> (gdb) complete set print elements
> set print elements NUMBER
> set print elements unlimited
> (gdb)
>    
> Adjust the testsuite accordingly.

Hi Maciej,

Thanks for looking at this!

> ---
> No change from v4.
> 
> New change in v4.
> ---
>   gdb/cli/cli-decode.c                |    2 ++
>   gdb/testsuite/gdb.base/settings.exp |    8 +++++++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 
> gdb-integer-complete-number.diff
> Index: src/gdb/cli/cli-decode.c
> ===================================================================
> --- src.orig/gdb/cli/cli-decode.c
> +++ src/gdb/cli/cli-decode.c
> @@ -989,6 +989,8 @@ integer_unlimited_completer (struct cmd_
>         NULL,
>       };
>   
> +  if (*text == '\0')
> +    tracker.add_completion (make_unique_xstrdup ("NUMBER"));
>     complete_on_enum (tracker, keywords, text, word);
>   }

Seeing as the point of "complete_on_enum" is to add all keywords to the tracker, why not add "NUMBER" as a keyword? The only possible unfavorable side effect I can think of would be GDB completing N<TAB> to NUMBER, and personally I feel abstracting away how the option would be added is more important than not adding this completion option.

Another option would be having the function like this:

void
integer_unlimited_completer(...)
{
   static const char * const keywords [] =
     {
       "NUMBER",
       "unlimited",
       NULL
     }
   if (*text != '\0')
     complete_on_enum (tracker, keywords + 1, text, word);
   else
     complete_on_enum (tracker, keywords, text, word);
}

Or some more concise way of writing it.

>   
> Index: src/gdb/testsuite/gdb.base/settings.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.base/settings.exp
> +++ src/gdb/testsuite/gdb.base/settings.exp
> @@ -215,8 +215,14 @@ proc test-integer {variant} {
>   	test_gdb_complete_none \
>   	    "$set_cmd "
>       } else {
> +	test_gdb_complete_multiple "$set_cmd " "" "" {
> +	    "NUMBER"
> +	    "unlimited"
> +	}
> +	test_gdb_complete_none \
> +	    "$set_cmd 1"
>   	test_gdb_complete_unique \
> -	    "$set_cmd " \
> +	    "$set_cmd u" \
>   	    "$set_cmd unlimited"
>       }
>   
> 


Cheers!
Bruno Larsen