Re: [PATCH 10/22] Add optional styled argument to gdb.execute

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> [2021-03-06 18:40:50 +0100]:

> This makes it possible to use the colored output of commands, e.g. for
> a custom TuiWindow.
> 
> gdb/ChangeLog:
> 
> 2020-12-29  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* cli/cli-script.c (execute_control_commands_to_string): Use
> 	styled argument.
> 	* cli/cli-script.h (execute_control_commands_to_string): Add
> 	styled argument.
> 	* python/python.c (execute_gdb_command): Parse "styled" argument.
> 
> gdb/doc/ChangeLog:
> 
> 2020-12-29  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* python.texi (Basic Python): Document "styled" argument.
> ---
>  gdb/cli/cli-script.c |  4 ++--
>  gdb/cli/cli-script.h |  2 +-
>  gdb/doc/python.texi  |  6 +++++-
>  gdb/python/python.c  | 24 ++++++++++++++++++------
>  4 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index 9d0dd7796e0..8729c7c7132 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -422,13 +422,13 @@ execute_control_commands (struct command_line *cmdlines, int from_tty)
>  
>  std::string
>  execute_control_commands_to_string (struct command_line *commands,
> -				    int from_tty)
> +				    int from_tty, bool styled)
>  {
>    /* GDB_STDOUT should be better already restored during these
>       restoration callbacks.  */
>    set_batch_flag_and_restore_page_info save_page_info;
>  
> -  string_file str_file;
> +  string_file str_file (styled);
>  
>    {
>      current_uiout->redirect (&str_file);
> diff --git a/gdb/cli/cli-script.h b/gdb/cli/cli-script.h
> index 8624bf55839..57cfff4ba33 100644
> --- a/gdb/cli/cli-script.h
> +++ b/gdb/cli/cli-script.h
> @@ -135,7 +135,7 @@ extern void execute_control_commands (struct command_line *cmdlines,
>     will be temporarily set to true.  */
>  
>  extern std::string execute_control_commands_to_string
> -    (struct command_line *commands, int from_tty);
> +    (struct command_line *commands, int from_tty, bool styled = false);

The comment immediately above this function needs updating.

>  
>  /* Exported to gdb/breakpoint.c */
>  
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 97a28d054fa..565e87d0784 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -217,7 +217,7 @@ A string containing the python directory (@pxref{Python}).
>  @end defvar
>  
>  @findex gdb.execute
> -@defun gdb.execute (command @r{[}, from_tty @r{[}, to_string@r{]]})
> +@defun gdb.execute (command @r{[}, from_tty @r{[}, to_string @r{[}, styled@r{]]]})
>  Evaluate @var{command}, a string, as a @value{GDBN} CLI command.
>  If a GDB exception happens while @var{command} runs, it is
>  translated as described in @ref{Exception Handling,,Exception Handling}.
> @@ -234,6 +234,10 @@ returned as a string.  The default is @code{False}, in which case the
>  return value is @code{None}.  If @var{to_string} is @code{True}, the
>  @value{GDBN} virtual terminal will be temporarily set to unlimited width
>  and height, and its pagination will be disabled; @pxref{Screen Size}.
> +
> +If both @var{to_string} and @var{styled} are @code{True}, then the returned
> +string also contains the ANSI terminal escape styling sequences used for
> +colored output.

I think this leaves out a lot of information.  You need to mention
that `styled` is optional, and that the default is false.  You should
also be explicit that when `to_string` is false `styled` is ignored,
something like:

  The optional argument @var{styled} defaults to @code{True}.  If both
  @var{to_string} and @var{styled} are @code{True}, then the returned
  string also contains the ANSI terminal escape styling sequences used
  for colored output.  If @var{to_string} is @code{False} then
  @var{styled} is ignored.

>  @end defun
>  
>  @findex gdb.breakpoints
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 009c0c4c6d7..e4e00e4627c 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -574,13 +574,15 @@ static PyObject *
>  execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>  {
>    const char *arg;
> -  PyObject *from_tty_obj = NULL, *to_string_obj = NULL;
> -  int from_tty, to_string;
> -  static const char *keywords[] = { "command", "from_tty", "to_string", NULL };
> +  PyObject *from_tty_obj = NULL, *to_string_obj = NULL, *styled_obj = NULL;
> +  int from_tty, to_string, styled;

That `from_tty` and `to_string` are int is a legacy of GDB's past. The
new `styled` should be bool.

Thanks,
Andrew

> +  static const char *keywords[] = { "command", "from_tty", "to_string",
> +				    "styled", NULL };
>  
> -  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!O!", keywords, &arg,
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!O!O!", keywords, &arg,
>  					&PyBool_Type, &from_tty_obj,
> -					&PyBool_Type, &to_string_obj))
> +					&PyBool_Type, &to_string_obj,
> +					&PyBool_Type, &styled_obj))
>      return NULL;
>  
>    from_tty = 0;
> @@ -601,6 +603,15 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>        to_string = cmp;
>      }
>  
> +  styled = 0;
> +  if (styled_obj)
> +    {
> +      int cmp = PyObject_IsTrue (styled_obj);
> +      if (cmp < 0)
> +	return NULL;
> +      styled = cmp;
> +    }
> +
>    std::string to_string_res;
>  
>    scoped_restore preventer = prevent_dont_repeat ();
> @@ -638,7 +649,8 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>  
>  	if (to_string)
>  	  to_string_res = execute_control_commands_to_string (lines.get (),
> -							      from_tty);
> +							      from_tty,
> +							      styled);
>  	else
>  	  execute_control_commands (lines.get (), from_tty);
>        }
> -- 
> 2.30.1
>