Re: [RFA] completion in command definition

[Pedro Alves <palves at redhat dot com>]

On 01/31/2017 02:29 PM, Jerome Guitton wrote:
> Hi Pedro,
> 
> Pedro Alves (palves@redhat.com):
>> > OK (but please remember to edit out the question and "attachment"
>> > from the commit log).
> I've integrated your comments; I can push the patch in attachment
> next week, if no objection.

A few minor comments below.

>  
> +/* Return non-zero if CMD's name is NAME.  */

s/non-zero/true/.

> +
> +static bool
> +command_name_equals (struct cmd_list_element *cmd, const char *name)
> +{
> +  return (cmd != NULL
> +	  && cmd != CMD_LIST_AMBIGUOUS
> +	  && strcmp (cmd->name, name) == 0);
> +}
> +
> +/* Given an input line P, skip the command and return a pointer to the
> +   first argument.  */
> +
> +static const char *
> +line_first_arg (const char *p)
> +{
> +  const char *first_arg = p + find_command_name_length (p);
> +
> +  return skip_spaces_const (first_arg); 
> +}
> +
>  /* Process one input line.  If the command is an "end", return such an
>     indication to the caller.  If PARSE_COMMANDS is true, strip leading
>     whitespace (trailing whitespace is always stripped) in the line,
> @@ -938,9 +959,14 @@ process_next_line (char *p, struct command_line **command, int parse_commands,
>       We also permit whitespace before end and after.  */
>    if (p_end - p_start == 3 && startswith (p_start, "end"))
>      return end_command;
> -  
> +
>    if (parse_commands)
>      {
> +      /* Resolve command abbreviations (e.g. 'ws' for 'while-stepping').  */
> +      const char *cmd_name = p;
> +      struct cmd_list_element *cmd =
> +	lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1);

"=" goes on the next line:

      struct cmd_list_element *cmd 
        = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1);


>  
> +# Verify that the command parser properly handles command abbreviations.
> +with_test_prefix "command abbreviations in define" {
> +  set test "define user command: breakmain"
> +  gdb_test_multiple "define breakmain" "$test" {
> +      -re "Type commands for definition of \"breakmain\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	  pass "$test"
> +	  set test "send body of breakmain"
> +	  gdb_test_multiple "break main\ncommand\necho\nend\nend" "$test"  {
> +	      -re "$gdb_prompt $"\
> +		  {pass "$test"}
> +	  }
> +      }
> +  }
> +
> +  gdb_test "breakmain" ".*Breakpoint .*" "run user command"
> +
> +  # If GDB fails to interpret properly the abbrev "command", the last "end"
> +  # will be missing. Issue it to avoid a desync that would break the other
> +  # tests in this file.

Double space after period.

> +  gdb_test "end" ".*This command cannot be used at the top level.*" "additional end command"

Too-long line.  The initial ".*" in "This command" is not
necessary; it's implicit.

> +
> +  gdb_test "info break \$bpnum" \
> +    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
> +\[0-9\]+\[\t \]+breakpoint     keep y.* in main at .*
> +\[\t \]+echo.*" 

I think we need to use use multi_line, to avoid problems with
"\n" vs "\r" vs "\r\n".

"info break shows echo command"
> +}


> +
> +
>  # Verify that the command parser doesn't require a space after an 'while'
>  # command in a user defined function.
>  #
> 

Thanks,
Pedro Alves