Re: [PATCH 2/3] Error out immediatly when using if command without args in command list

[Pedro Alves <palves@redhat.com>]

On 09/02/2017 10:36 PM, Simon Marchi wrote:
> When using "if" (or while) without args directly on gdb's command line,
> you get this:
> 
>   (gdb) if
>   if/while commands require arguments
> 
> When doing the same when entering a command list, you only get an error
> when the command is executed, when parse_exp_in_context_1 fails to
> evaluate the expression.
> 
>   (gdb) define foo
>   Type commands for definition of "foo".
>   End with a line saying just "end".
>   >if
>    >end
>   >end
>   (gdb) foo
>   Argument required (expression to compute).
> 
> I think it would make more sense to error out when inputting the command
> list directly:
> 
>   (gdb) define foo
>   Type commands for definition of "foo".
>   End with a line saying just "end".
>   >if
>   if/while commands require arguments.
> 
> The only required change is to check whether args is an empty string in
> build_command_line.
> 

LGTM.  Tiny nit further below.

BTW, as a potential improvement, we could consider also not
canceling the whole command definition, but instead go back to
expecting  another line.  It's a bit annoying to have to type
everything from scratch.  I've run into that occasionally with
tracepoints, like:

 (gdb) trace foo
 (gdb) actions
 Enter actions for tracepoint 1, one per line.
 End with a line saying just "end".
 >collect ...
 >collect ...
 > #... several lines later:
 >endd  # whoops, a typo.
 `endd' is not a tracepoint action, or is ambiguous.
 (gdb) # bah, have to start over.

Instead of:

 (gdb) trace foo
 (gdb) actions
 Enter actions for tracepoint 1, one per line.
 End with a line saying just "end".
 >collect ...
 >collect ...
 > #... several lines later:
 >endd
 `endd' is not a tracepoint action, or is ambiguous.
 >end
(gdb)

The same safety net applied to if/while typos might be useful.
Just an idea.

> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -147,7 +147,8 @@ build_command_line (enum command_control_type type, const char *args)
>  {
>    struct command_line *cmd;
>  
> -  if (args == NULL && (type == if_control || type == while_control))
> +  if ((args == NULL || strlen (args) == 0)
> +      && (type == if_control || type == while_control))
>      error (_("if/while commands require arguments."));
>    gdb_assert (args != NULL);

Nit: might not make a difference with modern compilers, though
the canonical way to check for entry string would be:

  *args == '\0'

Thanks,
Pedro Alves