[PATCHv2] Add optional styled argument to gdb.execute

[PATCHv2] Add optional styled argument to gdb.execute

[Hannes Domani]

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.
---
v2:
- More detailed description of styled in documantation.
- Changed styled variable to bool.
---
 gdb/cli/cli-script.c |  4 ++--
 gdb/cli/cli-script.h |  6 ++++--
 gdb/doc/python.texi  |  8 +++++++-
 gdb/python/python.c  | 23 ++++++++++++++++++-----
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index eb8853a5e64..c59beb5a158 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..3871a35d8f7 100644
--- a/gdb/cli/cli-script.h
+++ b/gdb/cli/cli-script.h
@@ -132,10 +132,12 @@ extern void execute_control_commands (struct command_line *cmdlines,
 
 /* Run execute_control_commands for COMMANDS.  Capture its output into
    the returned string, do not display it to the screen.  BATCH_FLAG
-   will be temporarily set to true.  */
+   will be temporarily set to true.  With STYLED=true the returned string
+   also contains the ANSI terminal escape styling sequences used for
+   colored output.  */
 
 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);
 
 /* Exported to gdb/breakpoint.c */
 
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index fa99892e1e0..7d1cc69f51a 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -273,7 +273,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}.
@@ -290,6 +290,12 @@ 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}.
+
+The optional argument @var{styled} defaults to @code{False}.  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 4cea83c3837..1444dc39909 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -573,13 +573,16 @@ static PyObject *
 execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
 {
   const char *arg;
-  PyObject *from_tty_obj = NULL, *to_string_obj = NULL;
+  PyObject *from_tty_obj = NULL, *to_string_obj = NULL, *styled_obj = NULL;
   int from_tty, to_string;
-  static const char *keywords[] = { "command", "from_tty", "to_string", NULL };
+  bool styled;
+  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;
@@ -600,6 +603,15 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
       to_string = cmp;
     }
 
+  styled = false;
+  if (styled_obj)
+    {
+      int cmp = PyObject_IsTrue (styled_obj);
+      if (cmp < 0)
+	return NULL;
+      styled = cmp != 0;
+    }
+
   std::string to_string_res;
 
   scoped_restore preventer = prevent_dont_repeat ();
@@ -637,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.31.1

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Eli Zaretskii]

> Date: Wed, 19 May 2021 19:26:49 +0200
> From: Hannes Domani via Gdb-patches <gdb-patches@sourceware.org>
> 
> 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.

OK for the python.texi part.

Thanks.

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Simon Marchi]

On 2021-05-19 1:26 p.m., Hannes Domani via Gdb-patches wrote:
> This makes it possible to use the colored output of commands, e.g. for
> a custom TuiWindow.

Just for not-painting-ourselves-in-a-corner purposes, do you foresee
that the "styled" parameter could be used in more cases in the future?

For example, to control styling even when to_string=False (outputting to
standard output).  In particular, I'm wondering if it should be some
kind of tri-bool (True/False/None).  None (the default) would mean "do
whatever the default is", based on "set style enabled", whether stdout
is an interactive terminal, and whatnot.  True/False would force it to
on and off.

Simon

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Hannes Domani]

 Am Donnerstag, 20. Mai 2021, 17:18:25 MESZ hat Simon Marchi <simon.marchi@polymtl.ca> Folgendes geschrieben:

> On 2021-05-19 1:26 p.m., Hannes Domani via Gdb-patches wrote:
>
> > This makes it possible to use the colored output of commands, e.g. for
> > a custom TuiWindow.
>
>
> Just for not-painting-ourselves-in-a-corner purposes, do you foresee
> that the "styled" parameter could be used in more cases in the future?
>
> For example, to control styling even when to_string=False (outputting to
> standard output).  In particular, I'm wondering if it should be some
> kind of tri-bool (True/False/None).  None (the default) would mean "do
> whatever the default is", based on "set style enabled", whether stdout
> is an interactive terminal, and whatnot.  True/False would force it to
> on and off.

When I call gdb.execute() with to_string=False, the output to stdout is colored,
so do you mean styled=False should disable the colors?


Hannes

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Simon Marchi]

On 2021-05-20 11:42 a.m., Hannes Domani wrote:
> When I call gdb.execute() with to_string=False, the output to stdout is colored,
> so do you mean styled=False should disable the colors?

Yes.  Well, I'm asking whether we might want that in the future (to be
able to control styling when using gdb.execute with to_string=False).
If we might want that in the future, I think it would be natural to use
the "styled" parameter for this.  And so I ask whether a simple
True/False boolean will be enough for this, given that True/False would
(presumably) force the styling to on or off.  Will there be a need for a
third (default) value to say "do whatever the default is"?

Simon

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Hannes Domani]

 Am Donnerstag, 20. Mai 2021, 18:12:47 MESZ hat Simon Marchi <simon.marchi@polymtl.ca> Folgendes geschrieben:

> On 2021-05-20 11:42 a.m., Hannes Domani wrote:
> > When I call gdb.execute() with to_string=False, the output to stdout is colored,
> > so do you mean styled=False should disable the colors?
>
> Yes.  Well, I'm asking whether we might want that in the future (to be
> able to control styling when using gdb.execute with to_string=False).
> If we might want that in the future, I think it would be natural to use
> the "styled" parameter for this.  And so I ask whether a simple
> True/False boolean will be enough for this, given that True/False would
> (presumably) force the styling to on or off.  Will there be a need for a
> third (default) value to say "do whatever the default is"?

I'm not sure it's necessary, anyone who wants styling enabled/disabled,
can call gdb.execute(to_string=True) with styled True/False, and just print
the output string.


Hannes

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Marco Barisione]

On 20 May 2021, at 17:34, Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> wrote:
> Am Donnerstag, 20. Mai 2021, 18:12:47 MESZ hat Simon Marchi <simon.marchi@polymtl.ca> Folgendes geschrieben:
> 
>> On 2021-05-20 11:42 a.m., Hannes Domani wrote:
>>> When I call gdb.execute() with to_string=False, the output to stdout is colored,
>>> so do you mean styled=False should disable the colors?
>> 
>> Yes.  Well, I'm asking whether we might want that in the future (to be
>> able to control styling when using gdb.execute with to_string=False).
>> If we might want that in the future, I think it would be natural to use
>> the "styled" parameter for this.  And so I ask whether a simple
>> True/False boolean will be enough for this, given that True/False would
>> (presumably) force the styling to on or off.  Will there be a need for a
>> third (default) value to say "do whatever the default is"?
> 
> I'm not sure it's necessary, anyone who wants styling enabled/disabled,
> can call gdb.execute(to_string=True) with styled True/False, and just print
> the output string.

This is not the same as the command may take time, so you don’t want to
delay the printing until the command is finished.


In the project I work on, I use an older version of this patch (which,
by the way, works great).  My use case is to get the output of the
command, do some stuff to it and print it back.

Simon,  I’m not sure I understand correctly your proposal about
using None/True/False.  Do you mean that when styled=True the output
would be styled even if you did "set style enabled off"?
I think this would make writing code more complicated as there would
be no easy way to achieve what I think is the patch’s original purpose,
i.e. just get a string containing what GDB would have printed to the
terminal.

On the other hand, disabling styling temporarily is trivial.  I use
code similar to this in the project I work on:

    @contextlib.contextmanager
    def style_disabled():
        enabled_before = gdb.parameter("style enabled")

        try:
            gdb.execute("set style enabled off")
            yield
        finally:
            if enabled_before:
                gdb.execute("set style enabled on")


    with style_disabled():
        gdb.execute("backtrace")


-- 
Marco Barisione

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Simon Marchi]

On 2021-05-20 1:03 p.m., Marco Barisione wrote:
>> I'm not sure it's necessary, anyone who wants styling enabled/disabled,
>> can call gdb.execute(to_string=True) with styled True/False, and just print
>> the output string.
> 
> This is not the same as the command may take time, so you don’t want to
> delay the printing until the command is finished.
> 
> 
> In the project I work on, I use an older version of this patch (which,
> by the way, works great).  My use case is to get the output of the
> command, do some stuff to it and print it back.
> 
> Simon,  I’m not sure I understand correctly your proposal about
> using None/True/False.  Do you mean that when styled=True the output
> would be styled even if you did "set style enabled off"?
> I think this would make writing code more complicated as there would
> be no easy way to achieve what I think is the patch’s original purpose,
> i.e. just get a string containing what GDB would have printed to the
> terminal.

style=None would be "do whatever GDB would do".  So yeah, style=True
would force the output to be styled even if "set style enabled" if off.
But I'm not sure I see a use case for that when outputting directly to
the terminal.

But you raise a good point, when outputting to a string, it would be
useful to have a "do whatever GDB would do" mode.  Is this what
style=True does?  This is not how it's documented at least.

Simon

Re: [PATCHv2] Add optional styled argument to gdb.execute

[Hannes Domani]

 Am Donnerstag, 20. Mai 2021, 19:49:32 MESZ hat Simon Marchi <simon.marchi@polymtl.ca> Folgendes geschrieben:

> On 2021-05-20 1:03 p.m., Marco Barisione wrote:
> >> I'm not sure it's necessary, anyone who wants styling enabled/disabled,
> >> can call gdb.execute(to_string=True) with styled True/False, and just print
> >> the output string.
> >
> > This is not the same as the command may take time, so you don’t want to
> > delay the printing until the command is finished.
> >
> >
> > In the project I work on, I use an older version of this patch (which,
> > by the way, works great).  My use case is to get the output of the
> > command, do some stuff to it and print it back.
> >
> > Simon,  I’m not sure I understand correctly your proposal about
> > using None/True/False.  Do you mean that when styled=True the output
> > would be styled even if you did "set style enabled off"?
> > I think this would make writing code more complicated as there would
> > be no easy way to achieve what I think is the patch’s original purpose,
> > i.e. just get a string containing what GDB would have printed to the
> > terminal.
>
> style=None would be "do whatever GDB would do".  So yeah, style=True
> would force the output to be styled even if "set style enabled" if off.
> But I'm not sure I see a use case for that when outputting directly to
> the terminal.
>
> But you raise a good point, when outputting to a string, it would be
> useful to have a "do whatever GDB would do" mode.  Is this what
> style=True does?  This is not how it's documented at least.

I tried it out now (I didn't think of this combination).
With "set style enabled off", the output of gdb.execute() never has colors,
no matter if styled=True is used.
So yes, actually styled=True "does what GDB would do".


Hannes