Re: [PATCH] gdb: multi-line support for "document" command

Simon Marchi
2020-12-01T15:22:22Z

On 2020-11-28 9:24 a.m., Rae Kim via Gdb-patches wrote:
> "document" command executed in python, gdb.execute("document
> <comname>\n...\nend\n"), will wait for user input. Python extension stops
> working from that point.
>
> "define" command works well with gdb.execute(). Looks like, when multi-line
> support was first introdued in commit 56bcdbea2b, "document" was left out.
>
> I checked the way "define" handles multi-line mode. And applied it to
> "document" command.

Hi,

Thanks for the patch. I noted some small nits to fix below. Also, it
would be good to have a test for this. You can inspire yourself from
commit 56bcdbea2b and add a gdb.execute that defines a command, another
one that documents it, and then verify that they both worked.

Some tips for the ChangeLog:

> gdb/ChangeLog:
> 2020-11-28 Rae Kim <rae.kim@gmail.com>
>
> * gdb/cli/cli-script.c (do_document_command): Renamed from

Remove the leading "gdb/".

> * document_command() and handles multi-line input

Remove the leading "* ", and the parenthesis after document_command.

Use the present tense, imperative form: "Rename from document_command
and handle multi-line input". Don't forget a period at the end. Apply
below too.

> (multi_line_command_p): Handles document_control.
> (build_command_line): Likewise.
> (execute_control_command_1): Likewise.
> (process_next_line): Likewise.
> (document_command): Calls do_document_command with.
> * gdb/cli/cli-script.h (enum command_control_type): Added
> document_control.
> ---
> gdb/cli/cli-script.c | 42 ++++++++++++++++++++++++++++++++++++++----
> gdb/cli/cli-script.h | 1 +
> 2 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index afa44af95a..25807080e5 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -51,6 +51,9 @@ recurse_read_control_structure
> static void do_define_command (const char *comname, int from_tty,
> const counted_command_line *commands);
>
> +static void do_document_command (const char *comname, int from_tty,
> + const counted_command_line *commands);

The second line of parameters should be aligned with the first line of
parameters, so it's missing two columns. So it should be indented with
4 tabs and 1 space here.

> +
> static const char *read_next_line ();
>
> /* Level of control structure when reading. */
> @@ -71,6 +74,9 @@ static cmd_list_element *if_cmd_element = nullptr;
> /* Command element for the 'define' command. */
> static cmd_list_element *define_cmd_element = nullptr;
>
> +/* Command element for the 'document' command. */
> +static cmd_list_element *document_cmd_element = nullptr;
> +
> /* Structure for arguments to user defined functions. */
>
> class user_args
> @@ -139,6 +145,7 @@ multi_line_command_p (enum command_control_type type)
> case python_control:
> case guile_control:
> case define_control:
> + case document_control:
> return 1;
> default:
> return 0;
> @@ -159,6 +166,8 @@ build_command_line (enum command_control_type type, const char *args)
> error (_("while command requires an argument."));
> else if (type == define_control)
> error (_("define command requires an argument."));
> + else if (type == document_control)
> + error (_("document command requires an argument."));
> }
> gdb_assert (args != NULL);
>
> @@ -679,6 +688,12 @@ execute_control_command_1 (struct command_line *cmd, int from_tty)
> ret = simple_control;
> break;
>
> + case document_control:
> + print_command_trace ("document %s", cmd->line);
> + do_document_command (cmd->line, 0, &cmd->body_list_0);
> + ret = simple_control;
> + break;
> +
> case python_control:
> case guile_control:
> {
> @@ -1019,6 +1034,8 @@ process_next_line (const char *p, struct command_line **command,
> *command = build_command_line (commands_control, line_first_arg (p));
> else if (cmd == define_cmd_element)
> *command = build_command_line (define_control, line_first_arg (p));
> + else if (cmd == document_cmd_element)
> + *command = build_command_line (document_control, line_first_arg (p));
> else if (cmd == python_cmd_element && !inline_cmd)
> {
> /* Note that we ignore the inline "python command" form
> @@ -1508,8 +1525,13 @@ define_command (const char *comname, int from_tty)
> do_define_command (comname, from_tty, nullptr);
> }
>
> +/* Document a user-defined command. If COMMANDS is NULL, then this is a
> + top-level call and the document will be read using read_command_lines.
> + Otherwise, it is a "document" command in an existing command and the
> + commands are provided. */

Use two spaces after the last period too.

> static void
> -document_command (const char *comname, int from_tty)
> +do_document_command (const char *comname, int from_tty,
> + const counted_command_line *commands)

Align second line of parameters with the first one.

> {
> struct cmd_list_element *c, **list;
> const char *tem;
> @@ -1526,8 +1548,14 @@ document_command (const char *comname, int from_tty)
>
> std::string prompt = string_printf ("Type documentation for \"%s\".",
> comfull);

Move this string into the `if (commands == nullptr)`, as is done in
do_define_command.

> - counted_command_line doclines = read_command_lines (prompt.c_str (),
> +
> + counted_command_line doclines;
> +
> + if (commands == nullptr)
> + doclines = read_command_lines (prompt.c_str (),
> from_tty, 0, 0);
> + else
> + doclines = *commands;
>
> xfree ((char *) c->doc);
>
> @@ -1553,6 +1581,12 @@ document_command (const char *comname, int from_tty)
> }
> }
>
> +static void
> +document_command (const char *comname, int from_tty)
> +{
> + do_document_command (comname, from_tty, nullptr);
> +}
> +
> /* Implementation of the "define-prefix" command. */
>
> static void
> @@ -1676,11 +1710,11 @@ _initialize_cli_script ()
> /* "document", "define" and "define-prefix" use command_completer,
> as this helps the user to either type the command name and/or
> its prefixes. */
> - c = add_com ("document", class_support, document_command, _("\
> + document_cmd_element = add_com ("document", class_support, document_command, _("\

This line is now too long, please wrap it.

Simon