[RFA] Have 'thread|frame apply' style their output.
Commit Message
'thread|frame apply CMD' launches CMD so that CMD output goes to a string_file.
This patch ensures that string_file for such CMD output contains
style escape sequences that 'thread|frame apply' will later on
output on the real terminal, so as to have CMD output properly styled.
The idea is to have the class ui_file having overridable methods
to indicate that the output to this ui_file should be done using
'terminal' behaviour such as styling.
Then these methods are overriden in string_file so that a specially
constructed string_file will get output with style escape sequences.
After this patch, the output of CMD by thread|frame apply CMD is styled
similarly as when CMD is launched directly.
Note that string_file (term_out true) could also support wrapping,
but this is not done (yet?).
Tested on debian/amd64.
gdb/ChangeLog
2019-XX-YY Philippe Waroquiers <philippe.waroquiers@skynet.be>
Support style in 'frame|thread apply'
* gdbcmd.h (execute_command_to_string): New term_out parameter.
* record.c (record_start, record_stop): Update callers of
execute_command_to_string with false.
* ui-file.h (class ui_file): New term_out and can_emit_style_escape
methods.
(class string_file): New constructor with term_out parameter.
Override methods term_out and can_emit_style_escape. New member
term_out.
(class stdio_file): Override can_emit_style_escape.
(class tee_file): Override term_out and can_emit_style_escape.
* utils.h (can_emit_style_escape): Remove.
* utils.c (can_emit_style_escape): Likewise.
Update all callers of can_emit_style_escape (SOMESTREAM) to
SOMESTREAM->can_emit_style_escape.
* source-cache.c (source_cache::get_source_lines): Likewise.
* stack.c (frame_apply_command_count): Call execute_command_to_string
passing the term_out characteristic of the current gdb_stdout.
* thread.c (thr_try_catch_cmd): Likewise.
* top.c (execute_command_to_string): pass term_out parameter
to construct the string_file for the command output.
* ui-file.c (term_cli_styling): New function (most code moved
from utils.c can_emit_style_escape.
(string_file::string_file, string_file::can_emit_style_escape,
stdio_file::can_emit_style_escape, tee_file::term_out,
tee_file::can_emit_style_escape): New functions.
---
gdb/gdbcmd.h | 7 ++++-
gdb/record.c | 12 ++++----
gdb/source-cache.c | 2 +-
gdb/stack.c | 3 +-
gdb/thread.c | 3 +-
gdb/top.c | 5 ++--
gdb/ui-file.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
gdb/ui-file.h | 25 ++++++++++++++++-
gdb/utils.c | 37 ++++--------------------
gdb/utils.h | 4 ---
10 files changed, 120 insertions(+), 48 deletions(-)
Comments
Ping ?
Thanks
Philippe
On Sat, 2019-03-09 at 23:59 +0100, Philippe Waroquiers wrote:
> 'thread|frame apply CMD' launches CMD so that CMD output goes to a string_file.
> This patch ensures that string_file for such CMD output contains
> style escape sequences that 'thread|frame apply' will later on
> output on the real terminal, so as to have CMD output properly styled.
>
> The idea is to have the class ui_file having overridable methods
> to indicate that the output to this ui_file should be done using
> 'terminal' behaviour such as styling.
> Then these methods are overriden in string_file so that a specially
> constructed string_file will get output with style escape sequences.
>
> After this patch, the output of CMD by thread|frame apply CMD is styled
> similarly as when CMD is launched directly.
> Note that string_file (term_out true) could also support wrapping,
> but this is not done (yet?).
>
> Tested on debian/amd64.
>
> gdb/ChangeLog
> 2019-XX-YY Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> Support style in 'frame|thread apply'
>
> * gdbcmd.h (execute_command_to_string): New term_out parameter.
> * record.c (record_start, record_stop): Update callers of
> execute_command_to_string with false.
> * ui-file.h (class ui_file): New term_out and can_emit_style_escape
> methods.
> (class string_file): New constructor with term_out parameter.
> Override methods term_out and can_emit_style_escape. New member
> term_out.
> (class stdio_file): Override can_emit_style_escape.
> (class tee_file): Override term_out and can_emit_style_escape.
> * utils.h (can_emit_style_escape): Remove.
> * utils.c (can_emit_style_escape): Likewise.
> Update all callers of can_emit_style_escape (SOMESTREAM) to
> SOMESTREAM->can_emit_style_escape.
> * source-cache.c (source_cache::get_source_lines): Likewise.
> * stack.c (frame_apply_command_count): Call execute_command_to_string
> passing the term_out characteristic of the current gdb_stdout.
> * thread.c (thr_try_catch_cmd): Likewise.
> * top.c (execute_command_to_string): pass term_out parameter
> to construct the string_file for the command output.
> * ui-file.c (term_cli_styling): New function (most code moved
> from utils.c can_emit_style_escape.
> (string_file::string_file, string_file::can_emit_style_escape,
> stdio_file::can_emit_style_escape, tee_file::term_out,
> tee_file::can_emit_style_escape): New functions.
> ---
> gdb/gdbcmd.h | 7 ++++-
> gdb/record.c | 12 ++++----
> gdb/source-cache.c | 2 +-
> gdb/stack.c | 3 +-
> gdb/thread.c | 3 +-
> gdb/top.c | 5 ++--
> gdb/ui-file.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> gdb/ui-file.h | 25 ++++++++++++++++-
> gdb/utils.c | 37 ++++--------------------
> gdb/utils.h | 4 ---
> 10 files changed, 120 insertions(+), 48 deletions(-)
>
> diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
> index 4614ec748c..5d0e697d83 100644
> --- a/gdb/gdbcmd.h
> +++ b/gdb/gdbcmd.h
> @@ -133,7 +133,12 @@ extern struct cmd_list_element *showchecklist;
> extern struct cmd_list_element *save_cmdlist;
>
> extern void execute_command (const char *, int);
> -extern std::string execute_command_to_string (const char *p, int from_tty);
> +
> +/* Execute command P and returns its output. If TERM_OUT,
> + the output is built using terminal output behaviour such
> + as cli_styling. */
> +extern std::string execute_command_to_string (const char *p, int from_tty,
> + bool term_out);
>
> extern void print_command_line (struct command_line *, unsigned int,
> struct ui_file *);
> diff --git a/gdb/record.c b/gdb/record.c
> index 9c703646a7..828c19968a 100644
> --- a/gdb/record.c
> +++ b/gdb/record.c
> @@ -99,25 +99,25 @@ record_start (const char *method, const char *format, int from_tty)
> if (method == NULL)
> {
> if (format == NULL)
> - execute_command_to_string ("record", from_tty);
> + execute_command_to_string ("record", from_tty, false);
> else
> error (_("Invalid format."));
> }
> else if (strcmp (method, "full") == 0)
> {
> if (format == NULL)
> - execute_command_to_string ("record full", from_tty);
> + execute_command_to_string ("record full", from_tty, false);
> else
> error (_("Invalid format."));
> }
> else if (strcmp (method, "btrace") == 0)
> {
> if (format == NULL)
> - execute_command_to_string ("record btrace", from_tty);
> + execute_command_to_string ("record btrace", from_tty, false);
> else if (strcmp (format, "bts") == 0)
> - execute_command_to_string ("record btrace bts", from_tty);
> + execute_command_to_string ("record btrace bts", from_tty, false);
> else if (strcmp (format, "pt") == 0)
> - execute_command_to_string ("record btrace pt", from_tty);
> + execute_command_to_string ("record btrace pt", from_tty, false);
> else
> error (_("Invalid format."));
> }
> @@ -130,7 +130,7 @@ record_start (const char *method, const char *format, int from_tty)
> void
> record_stop (int from_tty)
> {
> - execute_command_to_string ("record stop", from_tty);
> + execute_command_to_string ("record stop", from_tty, false);
> }
>
> /* See record.h. */
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 097c8a3ae1..0643ef4343 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -174,7 +174,7 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
> return false;
>
> #ifdef HAVE_SOURCE_HIGHLIGHT
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> {
> const char *fullname = symtab_to_fullname (s);
>
> diff --git a/gdb/stack.c b/gdb/stack.c
> index bce8d58f54..63cff3f171 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -2703,7 +2703,8 @@ frame_apply_command_count (const char *which_command,
> set to the selected frame. */
> scoped_restore_current_thread restore_fi_current_frame;
>
> - cmd_result = execute_command_to_string (cmd, from_tty);
> + cmd_result = execute_command_to_string
> + (cmd, from_tty, gdb_stdout->term_out ());
> }
> fi = get_selected_frame (_("frame apply "
> "unable to get selected frame."));
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 6c23252964..99ed6efdee 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1460,7 +1460,8 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
> switch_to_thread (thr);
> TRY
> {
> - std::string cmd_result = execute_command_to_string (cmd, from_tty);
> + std::string cmd_result = execute_command_to_string
> + (cmd, from_tty, gdb_stdout->term_out ());
> if (!flags.silent || cmd_result.length () > 0)
> {
> if (!flags.quiet)
> diff --git a/gdb/top.c b/gdb/top.c
> index 4065df7081..f21d9b5e43 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -657,7 +657,8 @@ execute_command (const char *p, int from_tty)
> temporarily set to true. */
>
> std::string
> -execute_command_to_string (const char *p, int from_tty)
> +execute_command_to_string (const char *p, int from_tty,
> + bool term_out)
> {
> /* GDB_STDOUT should be better already restored during these
> restoration callbacks. */
> @@ -665,7 +666,7 @@ execute_command_to_string (const char *p, int from_tty)
>
> scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
>
> - string_file str_file;
> + string_file str_file (term_out);
>
> {
> current_uiout->redirect (&str_file);
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 77f6b31ce4..ad7a6df51c 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -101,6 +101,32 @@ ui_file_isatty (struct ui_file *file)
> return file->isatty ();
> }
>
> +/* true if the gdb terminal supports styling, and styling is enabled. */
> +static bool
> +term_cli_styling (void)
> +{
> + extern int cli_styling;
> +
> + if (!cli_styling)
> + return false;
> +
> + const char *term = getenv ("TERM");
> + /* Windows doesn't by default define $TERM, but can support styles
> + regardless. */
> +#ifndef _WIN32
> + if (term == nullptr || !strcmp (term, "dumb"))
> + return false;
> +#else
> + /* But if they do define $TERM, let us behave the same as on Posix
> + platforms, for the benefit of programs which invoke GDB as their
> + back-end. */
> + if (term && !strcmp (term, "dumb"))
> + return false;
> +#endif
> + return true;
> +}
> +
> +
> void
> ui_file_write (struct ui_file *file,
> const char *buf,
> @@ -131,6 +157,16 @@ fputs_unfiltered (const char *buf, struct ui_file *file)
>
>
>
> +string_file::string_file ()
> +{
> + m_term_out = false;
> +}
> +
> +string_file::string_file (bool term_out)
> +{
> + m_term_out = term_out;
> +}
> +
> string_file::~string_file ()
> {}
>
> @@ -140,6 +176,18 @@ string_file::write (const char *buf, long length_buf)
> m_string.append (buf, length_buf);
> }
>
> +bool
> +string_file::term_out ()
> +{
> + return m_term_out;
> +}
> +
> +bool
> +string_file::can_emit_style_escape ()
> +{
> + return m_term_out && term_cli_styling ();
> +}
> +
>
>
> stdio_file::stdio_file (FILE *file, bool close_p)
> @@ -255,6 +303,14 @@ stdio_file::isatty ()
> return ::isatty (m_fd);
> }
>
> +bool
> +stdio_file::can_emit_style_escape ()
> +{
> + return this == gdb_stdout
> + && this->isatty ()
> + && term_cli_styling ();
> +}
> +
>
>
> /* This is the implementation of ui_file method 'write' for stderr.
> @@ -332,3 +388,17 @@ tee_file::isatty ()
> {
> return m_one->isatty ();
> }
> +
> +bool
> +tee_file::term_out ()
> +{
> + return m_one->term_out ();
> +}
> +
> +bool
> +tee_file::can_emit_style_escape ()
> +{
> + return this == gdb_stdout
> + && m_one->term_out ()
> + && term_cli_styling ();
> +}
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 6e6ca1c9cd..1a688dc341 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -70,6 +70,14 @@ public:
> virtual bool isatty ()
> { return false; }
>
> + /* true indicates terminal output behaviour such as cli_styling. */
> + virtual bool term_out ()
> + { return isatty (); }
> +
> + /* true if ANSI escapes can be used on STREAM. */
> + virtual bool can_emit_style_escape ()
> + { return false; }
> +
> virtual void flush ()
> {}
> };
> @@ -109,7 +117,13 @@ extern int gdb_console_fputs (const char *, FILE *);
> class string_file : public ui_file
> {
> public:
> - string_file () {}
> + /* Construct a string_file to collect 'raw' output, i.e. without
> + 'terminal' behaviour such as cli_styling. */
> + string_file ();
> + /* If TERM_OUT, construct a string_file with terminal output behaviour
> + such as cli_styling)
> + else collect 'raw' output like the previous constructor. */
> + string_file (bool term_out);
> ~string_file () override;
>
> /* Override ui_file methods. */
> @@ -119,6 +133,9 @@ public:
> long read (char *buf, long length_buf) override
> { gdb_assert_not_reached ("a string_file is not readable"); }
>
> + bool term_out () override;
> + bool can_emit_style_escape () override;
> +
> /* string_file-specific public API. */
>
> /* Accesses the std::string containing the entire output collected
> @@ -145,6 +162,8 @@ public:
> private:
> /* The internal buffer. */
> std::string m_string;
> +
> + bool m_term_out;
> };
>
> /* A ui_file implementation that maps directly onto <stdio.h>'s FILE.
> @@ -183,6 +202,8 @@ public:
>
> bool isatty () override;
>
> + bool can_emit_style_escape () override;
> +
> private:
> /* Sets the internal stream to FILE, and saves the FILE's file
> descriptor in M_FD. */
> @@ -255,6 +276,8 @@ public:
> void puts (const char *) override;
>
> bool isatty () override;
> + bool term_out () override;
> + bool can_emit_style_escape () override;
> void flush () override;
>
> private:
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 840779a630..e35fa5798c 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1435,38 +1435,13 @@ emit_style_escape (const ui_file_style &style,
> fputs_unfiltered (style.to_ansi ().c_str (), stream);
> }
>
> -/* See utils.h. */
> -
> -bool
> -can_emit_style_escape (struct ui_file *stream)
> -{
> - if (stream != gdb_stdout
> - || !cli_styling
> - || !ui_file_isatty (stream))
> - return false;
> - const char *term = getenv ("TERM");
> - /* Windows doesn't by default define $TERM, but can support styles
> - regardless. */
> -#ifndef _WIN32
> - if (term == nullptr || !strcmp (term, "dumb"))
> - return false;
> -#else
> - /* But if they do define $TERM, let us behave the same as on Posix
> - platforms, for the benefit of programs which invoke GDB as their
> - back-end. */
> - if (term && !strcmp (term, "dumb"))
> - return false;
> -#endif
> - return true;
> -}
> -
> /* Set the current output style. This will affect future uses of the
> _filtered output functions. */
>
> static void
> set_output_style (struct ui_file *stream, const ui_file_style &style)
> {
> - if (!can_emit_style_escape (stream))
> + if (!stream->can_emit_style_escape ())
> return;
>
> /* Note that we don't pass STREAM here, because we want to emit to
> @@ -1479,7 +1454,7 @@ set_output_style (struct ui_file *stream, const ui_file_style &style)
> void
> reset_terminal_style (struct ui_file *stream)
> {
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> {
> /* Force the setting, regardless of what we think the setting
> might already be. */
> @@ -1504,7 +1479,7 @@ prompt_for_continue (void)
> bool disable_pagination = pagination_disabled_for_command;
>
> /* Clear the current styling. */
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> emit_style_escape (ui_file_style (), gdb_stdout);
>
> if (annotation_level > 1)
> @@ -1552,7 +1527,7 @@ prompt_for_continue (void)
> pagination_disabled_for_command = disable_pagination;
>
> /* Restore the current styling. */
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> emit_style_escape (applied_style);
>
> dont_repeat (); /* Forget prev cmd -- CR won't repeat it. */
> @@ -1805,7 +1780,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> lines_printed++;
> if (wrap_column)
> {
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> emit_style_escape (ui_file_style (), stream);
> /* If we aren't actually wrapping, don't output
> newline -- if chars_per_line is right, we
> @@ -1827,7 +1802,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> if (wrap_column)
> {
> fputs_unfiltered (wrap_indent, stream);
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> emit_style_escape (wrap_style, stream);
> /* FIXME, this strlen is what prevents wrap_indent from
> containing tabs. However, if we recurse to print it
> diff --git a/gdb/utils.h b/gdb/utils.h
> index f0cb48e7a5..76c10049a7 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -440,10 +440,6 @@ extern void fputs_styled (const char *linebuffer,
>
> extern void reset_terminal_style (struct ui_file *stream);
>
> -/* Return true if ANSI escapes can be used on STREAM. */
> -
> -extern bool can_emit_style_escape (struct ui_file *stream);
> -
> /* Display the host ADDR on STREAM formatted as ``0x%x''. */
> extern void gdb_print_host_address_1 (const void *addr, struct ui_file *stream);
>
Thanks
On Sat, 2019-03-09 at 23:59 +0100, Philippe Waroquiers wrote:
> 'thread|frame apply CMD' launches CMD so that CMD output goes to a string_file.
> This patch ensures that string_file for such CMD output contains
> style escape sequences that 'thread|frame apply' will later on
> output on the real terminal, so as to have CMD output properly styled.
>
> The idea is to have the class ui_file having overridable methods
> to indicate that the output to this ui_file should be done using
> 'terminal' behaviour such as styling.
> Then these methods are overriden in string_file so that a specially
> constructed string_file will get output with style escape sequences.
>
> After this patch, the output of CMD by thread|frame apply CMD is styled
> similarly as when CMD is launched directly.
> Note that string_file (term_out true) could also support wrapping,
> but this is not done (yet?).
>
> Tested on debian/amd64.
>
> gdb/ChangeLog
> 2019-XX-YY Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> Support style in 'frame|thread apply'
>
> * gdbcmd.h (execute_command_to_string): New term_out parameter.
> * record.c (record_start, record_stop): Update callers of
> execute_command_to_string with false.
> * ui-file.h (class ui_file): New term_out and can_emit_style_escape
> methods.
> (class string_file): New constructor with term_out parameter.
> Override methods term_out and can_emit_style_escape. New member
> term_out.
> (class stdio_file): Override can_emit_style_escape.
> (class tee_file): Override term_out and can_emit_style_escape.
> * utils.h (can_emit_style_escape): Remove.
> * utils.c (can_emit_style_escape): Likewise.
> Update all callers of can_emit_style_escape (SOMESTREAM) to
> SOMESTREAM->can_emit_style_escape.
> * source-cache.c (source_cache::get_source_lines): Likewise.
> * stack.c (frame_apply_command_count): Call execute_command_to_string
> passing the term_out characteristic of the current gdb_stdout.
> * thread.c (thr_try_catch_cmd): Likewise.
> * top.c (execute_command_to_string): pass term_out parameter
> to construct the string_file for the command output.
> * ui-file.c (term_cli_styling): New function (most code moved
> from utils.c can_emit_style_escape.
> (string_file::string_file, string_file::can_emit_style_escape,
> stdio_file::can_emit_style_escape, tee_file::term_out,
> tee_file::can_emit_style_escape): New functions.
> ---
> gdb/gdbcmd.h | 7 ++++-
> gdb/record.c | 12 ++++----
> gdb/source-cache.c | 2 +-
> gdb/stack.c | 3 +-
> gdb/thread.c | 3 +-
> gdb/top.c | 5 ++--
> gdb/ui-file.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> gdb/ui-file.h | 25 ++++++++++++++++-
> gdb/utils.c | 37 ++++--------------------
> gdb/utils.h | 4 ---
> 10 files changed, 120 insertions(+), 48 deletions(-)
>
> diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
> index 4614ec748c..5d0e697d83 100644
> --- a/gdb/gdbcmd.h
> +++ b/gdb/gdbcmd.h
> @@ -133,7 +133,12 @@ extern struct cmd_list_element *showchecklist;
> extern struct cmd_list_element *save_cmdlist;
>
> extern void execute_command (const char *, int);
> -extern std::string execute_command_to_string (const char *p, int from_tty);
> +
> +/* Execute command P and returns its output. If TERM_OUT,
> + the output is built using terminal output behaviour such
> + as cli_styling. */
> +extern std::string execute_command_to_string (const char *p, int from_tty,
> + bool term_out);
>
> extern void print_command_line (struct command_line *, unsigned int,
> struct ui_file *);
> diff --git a/gdb/record.c b/gdb/record.c
> index 9c703646a7..828c19968a 100644
> --- a/gdb/record.c
> +++ b/gdb/record.c
> @@ -99,25 +99,25 @@ record_start (const char *method, const char *format, int from_tty)
> if (method == NULL)
> {
> if (format == NULL)
> - execute_command_to_string ("record", from_tty);
> + execute_command_to_string ("record", from_tty, false);
> else
> error (_("Invalid format."));
> }
> else if (strcmp (method, "full") == 0)
> {
> if (format == NULL)
> - execute_command_to_string ("record full", from_tty);
> + execute_command_to_string ("record full", from_tty, false);
> else
> error (_("Invalid format."));
> }
> else if (strcmp (method, "btrace") == 0)
> {
> if (format == NULL)
> - execute_command_to_string ("record btrace", from_tty);
> + execute_command_to_string ("record btrace", from_tty, false);
> else if (strcmp (format, "bts") == 0)
> - execute_command_to_string ("record btrace bts", from_tty);
> + execute_command_to_string ("record btrace bts", from_tty, false);
> else if (strcmp (format, "pt") == 0)
> - execute_command_to_string ("record btrace pt", from_tty);
> + execute_command_to_string ("record btrace pt", from_tty, false);
> else
> error (_("Invalid format."));
> }
> @@ -130,7 +130,7 @@ record_start (const char *method, const char *format, int from_tty)
> void
> record_stop (int from_tty)
> {
> - execute_command_to_string ("record stop", from_tty);
> + execute_command_to_string ("record stop", from_tty, false);
> }
>
> /* See record.h. */
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 097c8a3ae1..0643ef4343 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -174,7 +174,7 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
> return false;
>
> #ifdef HAVE_SOURCE_HIGHLIGHT
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> {
> const char *fullname = symtab_to_fullname (s);
>
> diff --git a/gdb/stack.c b/gdb/stack.c
> index bce8d58f54..63cff3f171 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -2703,7 +2703,8 @@ frame_apply_command_count (const char *which_command,
> set to the selected frame. */
> scoped_restore_current_thread restore_fi_current_frame;
>
> - cmd_result = execute_command_to_string (cmd, from_tty);
> + cmd_result = execute_command_to_string
> + (cmd, from_tty, gdb_stdout->term_out ());
> }
> fi = get_selected_frame (_("frame apply "
> "unable to get selected frame."));
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 6c23252964..99ed6efdee 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1460,7 +1460,8 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
> switch_to_thread (thr);
> TRY
> {
> - std::string cmd_result = execute_command_to_string (cmd, from_tty);
> + std::string cmd_result = execute_command_to_string
> + (cmd, from_tty, gdb_stdout->term_out ());
> if (!flags.silent || cmd_result.length () > 0)
> {
> if (!flags.quiet)
> diff --git a/gdb/top.c b/gdb/top.c
> index 4065df7081..f21d9b5e43 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -657,7 +657,8 @@ execute_command (const char *p, int from_tty)
> temporarily set to true. */
>
> std::string
> -execute_command_to_string (const char *p, int from_tty)
> +execute_command_to_string (const char *p, int from_tty,
> + bool term_out)
> {
> /* GDB_STDOUT should be better already restored during these
> restoration callbacks. */
> @@ -665,7 +666,7 @@ execute_command_to_string (const char *p, int from_tty)
>
> scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
>
> - string_file str_file;
> + string_file str_file (term_out);
>
> {
> current_uiout->redirect (&str_file);
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 77f6b31ce4..ad7a6df51c 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -101,6 +101,32 @@ ui_file_isatty (struct ui_file *file)
> return file->isatty ();
> }
>
> +/* true if the gdb terminal supports styling, and styling is enabled. */
> +static bool
> +term_cli_styling (void)
> +{
> + extern int cli_styling;
> +
> + if (!cli_styling)
> + return false;
> +
> + const char *term = getenv ("TERM");
> + /* Windows doesn't by default define $TERM, but can support styles
> + regardless. */
> +#ifndef _WIN32
> + if (term == nullptr || !strcmp (term, "dumb"))
> + return false;
> +#else
> + /* But if they do define $TERM, let us behave the same as on Posix
> + platforms, for the benefit of programs which invoke GDB as their
> + back-end. */
> + if (term && !strcmp (term, "dumb"))
> + return false;
> +#endif
> + return true;
> +}
> +
> +
> void
> ui_file_write (struct ui_file *file,
> const char *buf,
> @@ -131,6 +157,16 @@ fputs_unfiltered (const char *buf, struct ui_file *file)
>
>
>
> +string_file::string_file ()
> +{
> + m_term_out = false;
> +}
> +
> +string_file::string_file (bool term_out)
> +{
> + m_term_out = term_out;
> +}
> +
> string_file::~string_file ()
> {}
>
> @@ -140,6 +176,18 @@ string_file::write (const char *buf, long length_buf)
> m_string.append (buf, length_buf);
> }
>
> +bool
> +string_file::term_out ()
> +{
> + return m_term_out;
> +}
> +
> +bool
> +string_file::can_emit_style_escape ()
> +{
> + return m_term_out && term_cli_styling ();
> +}
> +
>
>
> stdio_file::stdio_file (FILE *file, bool close_p)
> @@ -255,6 +303,14 @@ stdio_file::isatty ()
> return ::isatty (m_fd);
> }
>
> +bool
> +stdio_file::can_emit_style_escape ()
> +{
> + return this == gdb_stdout
> + && this->isatty ()
> + && term_cli_styling ();
> +}
> +
>
>
> /* This is the implementation of ui_file method 'write' for stderr.
> @@ -332,3 +388,17 @@ tee_file::isatty ()
> {
> return m_one->isatty ();
> }
> +
> +bool
> +tee_file::term_out ()
> +{
> + return m_one->term_out ();
> +}
> +
> +bool
> +tee_file::can_emit_style_escape ()
> +{
> + return this == gdb_stdout
> + && m_one->term_out ()
> + && term_cli_styling ();
> +}
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 6e6ca1c9cd..1a688dc341 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -70,6 +70,14 @@ public:
> virtual bool isatty ()
> { return false; }
>
> + /* true indicates terminal output behaviour such as cli_styling. */
> + virtual bool term_out ()
> + { return isatty (); }
> +
> + /* true if ANSI escapes can be used on STREAM. */
> + virtual bool can_emit_style_escape ()
> + { return false; }
> +
> virtual void flush ()
> {}
> };
> @@ -109,7 +117,13 @@ extern int gdb_console_fputs (const char *, FILE *);
> class string_file : public ui_file
> {
> public:
> - string_file () {}
> + /* Construct a string_file to collect 'raw' output, i.e. without
> + 'terminal' behaviour such as cli_styling. */
> + string_file ();
> + /* If TERM_OUT, construct a string_file with terminal output behaviour
> + such as cli_styling)
> + else collect 'raw' output like the previous constructor. */
> + string_file (bool term_out);
> ~string_file () override;
>
> /* Override ui_file methods. */
> @@ -119,6 +133,9 @@ public:
> long read (char *buf, long length_buf) override
> { gdb_assert_not_reached ("a string_file is not readable"); }
>
> + bool term_out () override;
> + bool can_emit_style_escape () override;
> +
> /* string_file-specific public API. */
>
> /* Accesses the std::string containing the entire output collected
> @@ -145,6 +162,8 @@ public:
> private:
> /* The internal buffer. */
> std::string m_string;
> +
> + bool m_term_out;
> };
>
> /* A ui_file implementation that maps directly onto <stdio.h>'s FILE.
> @@ -183,6 +202,8 @@ public:
>
> bool isatty () override;
>
> + bool can_emit_style_escape () override;
> +
> private:
> /* Sets the internal stream to FILE, and saves the FILE's file
> descriptor in M_FD. */
> @@ -255,6 +276,8 @@ public:
> void puts (const char *) override;
>
> bool isatty () override;
> + bool term_out () override;
> + bool can_emit_style_escape () override;
> void flush () override;
>
> private:
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 840779a630..e35fa5798c 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1435,38 +1435,13 @@ emit_style_escape (const ui_file_style &style,
> fputs_unfiltered (style.to_ansi ().c_str (), stream);
> }
>
> -/* See utils.h. */
> -
> -bool
> -can_emit_style_escape (struct ui_file *stream)
> -{
> - if (stream != gdb_stdout
> - || !cli_styling
> - || !ui_file_isatty (stream))
> - return false;
> - const char *term = getenv ("TERM");
> - /* Windows doesn't by default define $TERM, but can support styles
> - regardless. */
> -#ifndef _WIN32
> - if (term == nullptr || !strcmp (term, "dumb"))
> - return false;
> -#else
> - /* But if they do define $TERM, let us behave the same as on Posix
> - platforms, for the benefit of programs which invoke GDB as their
> - back-end. */
> - if (term && !strcmp (term, "dumb"))
> - return false;
> -#endif
> - return true;
> -}
> -
> /* Set the current output style. This will affect future uses of the
> _filtered output functions. */
>
> static void
> set_output_style (struct ui_file *stream, const ui_file_style &style)
> {
> - if (!can_emit_style_escape (stream))
> + if (!stream->can_emit_style_escape ())
> return;
>
> /* Note that we don't pass STREAM here, because we want to emit to
> @@ -1479,7 +1454,7 @@ set_output_style (struct ui_file *stream, const ui_file_style &style)
> void
> reset_terminal_style (struct ui_file *stream)
> {
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> {
> /* Force the setting, regardless of what we think the setting
> might already be. */
> @@ -1504,7 +1479,7 @@ prompt_for_continue (void)
> bool disable_pagination = pagination_disabled_for_command;
>
> /* Clear the current styling. */
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> emit_style_escape (ui_file_style (), gdb_stdout);
>
> if (annotation_level > 1)
> @@ -1552,7 +1527,7 @@ prompt_for_continue (void)
> pagination_disabled_for_command = disable_pagination;
>
> /* Restore the current styling. */
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> emit_style_escape (applied_style);
>
> dont_repeat (); /* Forget prev cmd -- CR won't repeat it. */
> @@ -1805,7 +1780,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> lines_printed++;
> if (wrap_column)
> {
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> emit_style_escape (ui_file_style (), stream);
> /* If we aren't actually wrapping, don't output
> newline -- if chars_per_line is right, we
> @@ -1827,7 +1802,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> if (wrap_column)
> {
> fputs_unfiltered (wrap_indent, stream);
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> emit_style_escape (wrap_style, stream);
> /* FIXME, this strlen is what prevents wrap_indent from
> containing tabs. However, if we recurse to print it
> diff --git a/gdb/utils.h b/gdb/utils.h
> index f0cb48e7a5..76c10049a7 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -440,10 +440,6 @@ extern void fputs_styled (const char *linebuffer,
>
> extern void reset_terminal_style (struct ui_file *stream);
>
> -/* Return true if ANSI escapes can be used on STREAM. */
> -
> -extern bool can_emit_style_escape (struct ui_file *stream);
> -
> /* Display the host ADDR on STREAM formatted as ``0x%x''. */
> extern void gdb_print_host_address_1 (const void *addr, struct ui_file *stream);
>
Thanks
Philippe
On Sat, 2019-03-09 at 23:59 +0100, Philippe Waroquiers wrote:
> 'thread|frame apply CMD' launches CMD so that CMD output goes to a string_file.
> This patch ensures that string_file for such CMD output contains
> style escape sequences that 'thread|frame apply' will later on
> output on the real terminal, so as to have CMD output properly styled.
>
> The idea is to have the class ui_file having overridable methods
> to indicate that the output to this ui_file should be done using
> 'terminal' behaviour such as styling.
> Then these methods are overriden in string_file so that a specially
> constructed string_file will get output with style escape sequences.
>
> After this patch, the output of CMD by thread|frame apply CMD is styled
> similarly as when CMD is launched directly.
> Note that string_file (term_out true) could also support wrapping,
> but this is not done (yet?).
>
> Tested on debian/amd64.
>
> gdb/ChangeLog
> 2019-XX-YY Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> Support style in 'frame|thread apply'
>
> * gdbcmd.h (execute_command_to_string): New term_out parameter.
> * record.c (record_start, record_stop): Update callers of
> execute_command_to_string with false.
> * ui-file.h (class ui_file): New term_out and can_emit_style_escape
> methods.
> (class string_file): New constructor with term_out parameter.
> Override methods term_out and can_emit_style_escape. New member
> term_out.
> (class stdio_file): Override can_emit_style_escape.
> (class tee_file): Override term_out and can_emit_style_escape.
> * utils.h (can_emit_style_escape): Remove.
> * utils.c (can_emit_style_escape): Likewise.
> Update all callers of can_emit_style_escape (SOMESTREAM) to
> SOMESTREAM->can_emit_style_escape.
> * source-cache.c (source_cache::get_source_lines): Likewise.
> * stack.c (frame_apply_command_count): Call execute_command_to_string
> passing the term_out characteristic of the current gdb_stdout.
> * thread.c (thr_try_catch_cmd): Likewise.
> * top.c (execute_command_to_string): pass term_out parameter
> to construct the string_file for the command output.
> * ui-file.c (term_cli_styling): New function (most code moved
> from utils.c can_emit_style_escape.
> (string_file::string_file, string_file::can_emit_style_escape,
> stdio_file::can_emit_style_escape, tee_file::term_out,
> tee_file::can_emit_style_escape): New functions.
> ---
> gdb/gdbcmd.h | 7 ++++-
> gdb/record.c | 12 ++++----
> gdb/source-cache.c | 2 +-
> gdb/stack.c | 3 +-
> gdb/thread.c | 3 +-
> gdb/top.c | 5 ++--
> gdb/ui-file.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> gdb/ui-file.h | 25 ++++++++++++++++-
> gdb/utils.c | 37 ++++--------------------
> gdb/utils.h | 4 ---
> 10 files changed, 120 insertions(+), 48 deletions(-)
>
> diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
> index 4614ec748c..5d0e697d83 100644
> --- a/gdb/gdbcmd.h
> +++ b/gdb/gdbcmd.h
> @@ -133,7 +133,12 @@ extern struct cmd_list_element *showchecklist;
> extern struct cmd_list_element *save_cmdlist;
>
> extern void execute_command (const char *, int);
> -extern std::string execute_command_to_string (const char *p, int from_tty);
> +
> +/* Execute command P and returns its output. If TERM_OUT,
> + the output is built using terminal output behaviour such
> + as cli_styling. */
> +extern std::string execute_command_to_string (const char *p, int from_tty,
> + bool term_out);
>
> extern void print_command_line (struct command_line *, unsigned int,
> struct ui_file *);
> diff --git a/gdb/record.c b/gdb/record.c
> index 9c703646a7..828c19968a 100644
> --- a/gdb/record.c
> +++ b/gdb/record.c
> @@ -99,25 +99,25 @@ record_start (const char *method, const char *format, int from_tty)
> if (method == NULL)
> {
> if (format == NULL)
> - execute_command_to_string ("record", from_tty);
> + execute_command_to_string ("record", from_tty, false);
> else
> error (_("Invalid format."));
> }
> else if (strcmp (method, "full") == 0)
> {
> if (format == NULL)
> - execute_command_to_string ("record full", from_tty);
> + execute_command_to_string ("record full", from_tty, false);
> else
> error (_("Invalid format."));
> }
> else if (strcmp (method, "btrace") == 0)
> {
> if (format == NULL)
> - execute_command_to_string ("record btrace", from_tty);
> + execute_command_to_string ("record btrace", from_tty, false);
> else if (strcmp (format, "bts") == 0)
> - execute_command_to_string ("record btrace bts", from_tty);
> + execute_command_to_string ("record btrace bts", from_tty, false);
> else if (strcmp (format, "pt") == 0)
> - execute_command_to_string ("record btrace pt", from_tty);
> + execute_command_to_string ("record btrace pt", from_tty, false);
> else
> error (_("Invalid format."));
> }
> @@ -130,7 +130,7 @@ record_start (const char *method, const char *format, int from_tty)
> void
> record_stop (int from_tty)
> {
> - execute_command_to_string ("record stop", from_tty);
> + execute_command_to_string ("record stop", from_tty, false);
> }
>
> /* See record.h. */
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 097c8a3ae1..0643ef4343 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -174,7 +174,7 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
> return false;
>
> #ifdef HAVE_SOURCE_HIGHLIGHT
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> {
> const char *fullname = symtab_to_fullname (s);
>
> diff --git a/gdb/stack.c b/gdb/stack.c
> index bce8d58f54..63cff3f171 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -2703,7 +2703,8 @@ frame_apply_command_count (const char *which_command,
> set to the selected frame. */
> scoped_restore_current_thread restore_fi_current_frame;
>
> - cmd_result = execute_command_to_string (cmd, from_tty);
> + cmd_result = execute_command_to_string
> + (cmd, from_tty, gdb_stdout->term_out ());
> }
> fi = get_selected_frame (_("frame apply "
> "unable to get selected frame."));
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 6c23252964..99ed6efdee 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1460,7 +1460,8 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
> switch_to_thread (thr);
> TRY
> {
> - std::string cmd_result = execute_command_to_string (cmd, from_tty);
> + std::string cmd_result = execute_command_to_string
> + (cmd, from_tty, gdb_stdout->term_out ());
> if (!flags.silent || cmd_result.length () > 0)
> {
> if (!flags.quiet)
> diff --git a/gdb/top.c b/gdb/top.c
> index 4065df7081..f21d9b5e43 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -657,7 +657,8 @@ execute_command (const char *p, int from_tty)
> temporarily set to true. */
>
> std::string
> -execute_command_to_string (const char *p, int from_tty)
> +execute_command_to_string (const char *p, int from_tty,
> + bool term_out)
> {
> /* GDB_STDOUT should be better already restored during these
> restoration callbacks. */
> @@ -665,7 +666,7 @@ execute_command_to_string (const char *p, int from_tty)
>
> scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
>
> - string_file str_file;
> + string_file str_file (term_out);
>
> {
> current_uiout->redirect (&str_file);
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 77f6b31ce4..ad7a6df51c 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -101,6 +101,32 @@ ui_file_isatty (struct ui_file *file)
> return file->isatty ();
> }
>
> +/* true if the gdb terminal supports styling, and styling is enabled. */
> +static bool
> +term_cli_styling (void)
> +{
> + extern int cli_styling;
> +
> + if (!cli_styling)
> + return false;
> +
> + const char *term = getenv ("TERM");
> + /* Windows doesn't by default define $TERM, but can support styles
> + regardless. */
> +#ifndef _WIN32
> + if (term == nullptr || !strcmp (term, "dumb"))
> + return false;
> +#else
> + /* But if they do define $TERM, let us behave the same as on Posix
> + platforms, for the benefit of programs which invoke GDB as their
> + back-end. */
> + if (term && !strcmp (term, "dumb"))
> + return false;
> +#endif
> + return true;
> +}
> +
> +
> void
> ui_file_write (struct ui_file *file,
> const char *buf,
> @@ -131,6 +157,16 @@ fputs_unfiltered (const char *buf, struct ui_file *file)
>
>
>
> +string_file::string_file ()
> +{
> + m_term_out = false;
> +}
> +
> +string_file::string_file (bool term_out)
> +{
> + m_term_out = term_out;
> +}
> +
> string_file::~string_file ()
> {}
>
> @@ -140,6 +176,18 @@ string_file::write (const char *buf, long length_buf)
> m_string.append (buf, length_buf);
> }
>
> +bool
> +string_file::term_out ()
> +{
> + return m_term_out;
> +}
> +
> +bool
> +string_file::can_emit_style_escape ()
> +{
> + return m_term_out && term_cli_styling ();
> +}
> +
>
>
> stdio_file::stdio_file (FILE *file, bool close_p)
> @@ -255,6 +303,14 @@ stdio_file::isatty ()
> return ::isatty (m_fd);
> }
>
> +bool
> +stdio_file::can_emit_style_escape ()
> +{
> + return this == gdb_stdout
> + && this->isatty ()
> + && term_cli_styling ();
> +}
> +
>
>
> /* This is the implementation of ui_file method 'write' for stderr.
> @@ -332,3 +388,17 @@ tee_file::isatty ()
> {
> return m_one->isatty ();
> }
> +
> +bool
> +tee_file::term_out ()
> +{
> + return m_one->term_out ();
> +}
> +
> +bool
> +tee_file::can_emit_style_escape ()
> +{
> + return this == gdb_stdout
> + && m_one->term_out ()
> + && term_cli_styling ();
> +}
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index 6e6ca1c9cd..1a688dc341 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -70,6 +70,14 @@ public:
> virtual bool isatty ()
> { return false; }
>
> + /* true indicates terminal output behaviour such as cli_styling. */
> + virtual bool term_out ()
> + { return isatty (); }
> +
> + /* true if ANSI escapes can be used on STREAM. */
> + virtual bool can_emit_style_escape ()
> + { return false; }
> +
> virtual void flush ()
> {}
> };
> @@ -109,7 +117,13 @@ extern int gdb_console_fputs (const char *, FILE *);
> class string_file : public ui_file
> {
> public:
> - string_file () {}
> + /* Construct a string_file to collect 'raw' output, i.e. without
> + 'terminal' behaviour such as cli_styling. */
> + string_file ();
> + /* If TERM_OUT, construct a string_file with terminal output behaviour
> + such as cli_styling)
> + else collect 'raw' output like the previous constructor. */
> + string_file (bool term_out);
> ~string_file () override;
>
> /* Override ui_file methods. */
> @@ -119,6 +133,9 @@ public:
> long read (char *buf, long length_buf) override
> { gdb_assert_not_reached ("a string_file is not readable"); }
>
> + bool term_out () override;
> + bool can_emit_style_escape () override;
> +
> /* string_file-specific public API. */
>
> /* Accesses the std::string containing the entire output collected
> @@ -145,6 +162,8 @@ public:
> private:
> /* The internal buffer. */
> std::string m_string;
> +
> + bool m_term_out;
> };
>
> /* A ui_file implementation that maps directly onto <stdio.h>'s FILE.
> @@ -183,6 +202,8 @@ public:
>
> bool isatty () override;
>
> + bool can_emit_style_escape () override;
> +
> private:
> /* Sets the internal stream to FILE, and saves the FILE's file
> descriptor in M_FD. */
> @@ -255,6 +276,8 @@ public:
> void puts (const char *) override;
>
> bool isatty () override;
> + bool term_out () override;
> + bool can_emit_style_escape () override;
> void flush () override;
>
> private:
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 840779a630..e35fa5798c 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1435,38 +1435,13 @@ emit_style_escape (const ui_file_style &style,
> fputs_unfiltered (style.to_ansi ().c_str (), stream);
> }
>
> -/* See utils.h. */
> -
> -bool
> -can_emit_style_escape (struct ui_file *stream)
> -{
> - if (stream != gdb_stdout
> - || !cli_styling
> - || !ui_file_isatty (stream))
> - return false;
> - const char *term = getenv ("TERM");
> - /* Windows doesn't by default define $TERM, but can support styles
> - regardless. */
> -#ifndef _WIN32
> - if (term == nullptr || !strcmp (term, "dumb"))
> - return false;
> -#else
> - /* But if they do define $TERM, let us behave the same as on Posix
> - platforms, for the benefit of programs which invoke GDB as their
> - back-end. */
> - if (term && !strcmp (term, "dumb"))
> - return false;
> -#endif
> - return true;
> -}
> -
> /* Set the current output style. This will affect future uses of the
> _filtered output functions. */
>
> static void
> set_output_style (struct ui_file *stream, const ui_file_style &style)
> {
> - if (!can_emit_style_escape (stream))
> + if (!stream->can_emit_style_escape ())
> return;
>
> /* Note that we don't pass STREAM here, because we want to emit to
> @@ -1479,7 +1454,7 @@ set_output_style (struct ui_file *stream, const ui_file_style &style)
> void
> reset_terminal_style (struct ui_file *stream)
> {
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> {
> /* Force the setting, regardless of what we think the setting
> might already be. */
> @@ -1504,7 +1479,7 @@ prompt_for_continue (void)
> bool disable_pagination = pagination_disabled_for_command;
>
> /* Clear the current styling. */
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> emit_style_escape (ui_file_style (), gdb_stdout);
>
> if (annotation_level > 1)
> @@ -1552,7 +1527,7 @@ prompt_for_continue (void)
> pagination_disabled_for_command = disable_pagination;
>
> /* Restore the current styling. */
> - if (can_emit_style_escape (gdb_stdout))
> + if (gdb_stdout->can_emit_style_escape ())
> emit_style_escape (applied_style);
>
> dont_repeat (); /* Forget prev cmd -- CR won't repeat it. */
> @@ -1805,7 +1780,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> lines_printed++;
> if (wrap_column)
> {
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> emit_style_escape (ui_file_style (), stream);
> /* If we aren't actually wrapping, don't output
> newline -- if chars_per_line is right, we
> @@ -1827,7 +1802,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
> if (wrap_column)
> {
> fputs_unfiltered (wrap_indent, stream);
> - if (can_emit_style_escape (stream))
> + if (stream->can_emit_style_escape ())
> emit_style_escape (wrap_style, stream);
> /* FIXME, this strlen is what prevents wrap_indent from
> containing tabs. However, if we recurse to print it
> diff --git a/gdb/utils.h b/gdb/utils.h
> index f0cb48e7a5..76c10049a7 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -440,10 +440,6 @@ extern void fputs_styled (const char *linebuffer,
>
> extern void reset_terminal_style (struct ui_file *stream);
>
> -/* Return true if ANSI escapes can be used on STREAM. */
> -
> -extern bool can_emit_style_escape (struct ui_file *stream);
> -
> /* Display the host ADDR on STREAM formatted as ``0x%x''. */
> extern void gdb_print_host_address_1 (const void *addr, struct ui_file *stream);
>
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
Philippe> 'thread|frame apply CMD' launches CMD so that CMD output goes to a string_file.
Philippe> This patch ensures that string_file for such CMD output contains
Philippe> style escape sequences that 'thread|frame apply' will later on
Philippe> output on the real terminal, so as to have CMD output properly styled.
Thanks for doing this.
Other than some nits, enumerated below, this looks good to me.
Philippe> +/* true if the gdb terminal supports styling, and styling is enabled. */
Philippe> +static bool
Philippe> +term_cli_styling (void)
No need for the "void" here.
Philippe> +string_file::string_file ()
Philippe> +{
Philippe> + m_term_out = false;
Philippe> +}
I think this should just be an inline initializer in the header.
Then this definition isn't needed at all.
Philippe> +string_file::string_file (bool term_out)
Philippe> +{
Philippe> + m_term_out = term_out;
Philippe> +}
This can also be in the header file.
Also it should use initializer syntax like
: m_term_out (term_out)
Philippe> +bool
Philippe> +string_file::term_out ()
Philippe> +{
Philippe> + return m_term_out;
Philippe> +}
Philippe> +
Philippe> +bool
Philippe> +string_file::can_emit_style_escape ()
Philippe> +{
Philippe> + return m_term_out && term_cli_styling ();
Philippe> +}
This should have comments saying /* See ui-file.h. */
Philippe> +bool
Philippe> +stdio_file::can_emit_style_escape ()
Likewise. There are some more instances.
Philippe> + return this == gdb_stdout
Philippe> + && this->isatty ()
Philippe> + && term_cli_styling ();
The expression needs parens and reindentation to GNU style.
Philippe> +bool
Philippe> +tee_file::can_emit_style_escape ()
Philippe> +{
Philippe> + return this == gdb_stdout
Philippe> + && m_one->term_out ()
Philippe> + && term_cli_styling ();
Indentation.
Philippe> + /* true indicates terminal output behaviour such as cli_styling. */
Philippe> + virtual bool term_out ()
Philippe> + { return isatty (); }
Philippe> +
Philippe> + /* true if ANSI escapes can be used on STREAM. */
Philippe> + virtual bool can_emit_style_escape ()
Philippe> + { return false; }
Should these be const?
Philippe> + /* If TERM_OUT, construct a string_file with terminal output behaviour
Philippe> + such as cli_styling)
Philippe> + else collect 'raw' output like the previous constructor. */
Philippe> + string_file (bool term_out);
Single-argument constructors should nearly always be "explicit".
Tom
On Thu, 2019-04-25 at 09:46 -0600, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
>
> Philippe> 'thread|frame apply CMD' launches CMD so that CMD output goes to a string_file.
> Philippe> This patch ensures that string_file for such CMD output contains
> Philippe> style escape sequences that 'thread|frame apply' will later on
> Philippe> output on the real terminal, so as to have CMD output properly styled.
>
> Thanks for doing this.
>
> Other than some nits, enumerated below, this looks good to me.
Thanks for the review. Pushed after fixing (for the "const?" comment,
I have added an explanation to clarify).
Philippe
>
> Philippe> +/* true if the gdb terminal supports styling, and styling is enabled. */
> Philippe> +static bool
> Philippe> +term_cli_styling (void)
>
> No need for the "void" here.
>
> Philippe> +string_file::string_file ()
> Philippe> +{
> Philippe> + m_term_out = false;
> Philippe> +}
>
> I think this should just be an inline initializer in the header.
> Then this definition isn't needed at all.
>
> Philippe> +string_file::string_file (bool term_out)
> Philippe> +{
> Philippe> + m_term_out = term_out;
> Philippe> +}
>
> This can also be in the header file.
> Also it should use initializer syntax like
>
> : m_term_out (term_out)
>
> Philippe> +bool
> Philippe> +string_file::term_out ()
> Philippe> +{
> Philippe> + return m_term_out;
> Philippe> +}
> Philippe> +
> Philippe> +bool
> Philippe> +string_file::can_emit_style_escape ()
> Philippe> +{
> Philippe> + return m_term_out && term_cli_styling ();
> Philippe> +}
>
> This should have comments saying /* See ui-file.h. */
>
> Philippe> +bool
> Philippe> +stdio_file::can_emit_style_escape ()
>
> Likewise. There are some more instances.
>
> Philippe> + return this == gdb_stdout
> Philippe> + && this->isatty ()
> Philippe> + && term_cli_styling ();
>
> The expression needs parens and reindentation to GNU style.
>
> Philippe> +bool
> Philippe> +tee_file::can_emit_style_escape ()
> Philippe> +{
> Philippe> + return this == gdb_stdout
> Philippe> + && m_one->term_out ()
> Philippe> + && term_cli_styling ();
>
> Indentation.
>
> Philippe> + /* true indicates terminal output behaviour such as cli_styling. */
> Philippe> + virtual bool term_out ()
> Philippe> + { return isatty (); }
> Philippe> +
> Philippe> + /* true if ANSI escapes can be used on STREAM. */
> Philippe> + virtual bool can_emit_style_escape ()
> Philippe> + { return false; }
>
> Should these be const?
>
> Philippe> + /* If TERM_OUT, construct a string_file with terminal output behaviour
> Philippe> + such as cli_styling)
> Philippe> + else collect 'raw' output like the previous constructor. */
> Philippe> + string_file (bool term_out);
>
> Single-argument constructors should nearly always be "explicit".
>
> Tom
On 2019-04-27 8:42 a.m., Philippe Waroquiers wrote:
> Thanks for the review. Pushed after fixing (for the "const?" comment,
> I have added an explanation to clarify).
>
> Philippe
Hi Philippe,
There is one more spot to update, which you'll find if you build with --with-guile:
CXX guile/guile.o
/home/simark/src/binutils-gdb/gdb/guile/guile.c: In lambda function:
/home/simark/src/binutils-gdb/gdb/guile/guile.c:310:62: error: too few arguments to function ‘std::__cxx11::string execute_command_to_string(const char*, int, bool)’
to_string_res = execute_command_to_string (command, from_tty);
^
In file included from /home/simark/src/binutils-gdb/gdb/guile/guile.c:29:
/home/simark/src/binutils-gdb/gdb/gdbcmd.h:140:20: note: declared here
extern std::string execute_command_to_string (const char *p, int from_tty,
^~~~~~~~~~~~~~~~~~~~~~~~~
I presume you'll want to pass false in this case, since it's specifically meant to
send the output to a string, where we don't want styling?
Simon
On Sat, 2019-04-27 at 15:19 -0400, Simon Marchi wrote:
> On 2019-04-27 8:42 a.m., Philippe Waroquiers wrote:
> > Thanks for the review. Pushed after fixing (for the "const?" comment,
> > I have added an explanation to clarify).
> >
> > Philippe
>
> Hi Philippe,
>
> There is one more spot to update, which you'll find if you build with --with-guile:
>
> CXX guile/guile.o
> /home/simark/src/binutils-gdb/gdb/guile/guile.c: In lambda function:
> /home/simark/src/binutils-gdb/gdb/guile/guile.c:310:62: error: too few arguments to function ‘std::__cxx11::string execute_command_to_string(const char*, int, bool)’
> to_string_res = execute_command_to_string (command, from_tty);
> ^
> In file included from /home/simark/src/binutils-gdb/gdb/guile/guile.c:29:
> /home/simark/src/binutils-gdb/gdb/gdbcmd.h:140:20: note: declared here
> extern std::string execute_command_to_string (const char *p, int from_tty,
> ^~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> I presume you'll want to pass false in this case, since it's specifically meant to
> send the output to a string, where we don't want styling?
Hello Simon,
I have just pushed a follow-up to fix the above.
At the same time, I saw that I forgot to copy/paste the ChangeLog
from the commit message to the ChangeLog file.
I fixed this also.
Sorry for the breakage, I have now added --with-guile as default.
Philippe
NB: this ChangeLog is a nightmare to produce (and maintain
for each version of a patch). And then still a final copy-paste
from commit log to the real file to not forget.
(to avoid merge conflicts).
The only good point of ChangeLog is that this is reminding me of
the 80s, when I was young, and there was no good source control system,
and we were all proudly indicating in each source file what we changed
at what date :).
This ChangeLog is just slowing down the free software development
by moving away resources to useless painful administrative activities.
This must have been invented by an evil person that hates free software :).
I think I will now type it directly (but at the end of
the file), and move it at the beginning of the file just before
pushing. At least, it reduces the nr of operations
(and I have a script I am running to automatically do some checks
before pushing, that I will make detect I forgot to move it).
On 2019-04-28 01:18, Philippe Waroquiers wrote:
> Hello Simon,
> I have just pushed a follow-up to fix the above.
Thanks!
> At the same time, I saw that I forgot to copy/paste the ChangeLog
> from the commit message to the ChangeLog file.
> I fixed this also.
>
> Sorry for the breakage, I have now added --with-guile as default.
>
> Philippe
>
> NB: this ChangeLog is a nightmare to produce (and maintain
> for each version of a patch). And then still a final copy-paste
> from commit log to the real file to not forget.
> (to avoid merge conflicts).
>
> The only good point of ChangeLog is that this is reminding me of
> the 80s, when I was young, and there was no good source control system,
> and we were all proudly indicating in each source file what we changed
> at what date :).
>
> This ChangeLog is just slowing down the free software development
> by moving away resources to useless painful administrative activities.
> This must have been invented by an evil person that hates free software
> :).
This is my personal opinion too.
There was a push to make them not mandatory for GNU projects. If you
have too much free time on your hand, you can read what lead up to and
the follow-up to this thread:
http://lists.gnu.org/archive/html/bug-standards/2017-11/msg00005.html
In short, some people claim that the git history is not sufficient to
fill the gap that would be left by removing ChangeLogs. So the
compromise is that if someone writes a good enough program to
automatically generate the ChangeLog entries, we can use that and not
have to write them by hand... I am not sure if such a program will
really ever be "good enough" so that we can use it, so we are kind of
stuck there.
> I think I will now type it directly (but at the end of
> the file), and move it at the beginning of the file just before
> pushing. At least, it reduces the nr of operations
> (and I have a script I am running to automatically do some checks
> before pushing, that I will make detect I forgot to move it).
@@ -133,7 +133,12 @@ extern struct cmd_list_element *showchecklist;
extern struct cmd_list_element *save_cmdlist;
extern void execute_command (const char *, int);
-extern std::string execute_command_to_string (const char *p, int from_tty);
+
+/* Execute command P and returns its output. If TERM_OUT,
+ the output is built using terminal output behaviour such
+ as cli_styling. */
+extern std::string execute_command_to_string (const char *p, int from_tty,
+ bool term_out);
extern void print_command_line (struct command_line *, unsigned int,
struct ui_file *);
@@ -99,25 +99,25 @@ record_start (const char *method, const char *format, int from_tty)
if (method == NULL)
{
if (format == NULL)
- execute_command_to_string ("record", from_tty);
+ execute_command_to_string ("record", from_tty, false);
else
error (_("Invalid format."));
}
else if (strcmp (method, "full") == 0)
{
if (format == NULL)
- execute_command_to_string ("record full", from_tty);
+ execute_command_to_string ("record full", from_tty, false);
else
error (_("Invalid format."));
}
else if (strcmp (method, "btrace") == 0)
{
if (format == NULL)
- execute_command_to_string ("record btrace", from_tty);
+ execute_command_to_string ("record btrace", from_tty, false);
else if (strcmp (format, "bts") == 0)
- execute_command_to_string ("record btrace bts", from_tty);
+ execute_command_to_string ("record btrace bts", from_tty, false);
else if (strcmp (format, "pt") == 0)
- execute_command_to_string ("record btrace pt", from_tty);
+ execute_command_to_string ("record btrace pt", from_tty, false);
else
error (_("Invalid format."));
}
@@ -130,7 +130,7 @@ record_start (const char *method, const char *format, int from_tty)
void
record_stop (int from_tty)
{
- execute_command_to_string ("record stop", from_tty);
+ execute_command_to_string ("record stop", from_tty, false);
}
/* See record.h. */
@@ -174,7 +174,7 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
return false;
#ifdef HAVE_SOURCE_HIGHLIGHT
- if (can_emit_style_escape (gdb_stdout))
+ if (gdb_stdout->can_emit_style_escape ())
{
const char *fullname = symtab_to_fullname (s);
@@ -2703,7 +2703,8 @@ frame_apply_command_count (const char *which_command,
set to the selected frame. */
scoped_restore_current_thread restore_fi_current_frame;
- cmd_result = execute_command_to_string (cmd, from_tty);
+ cmd_result = execute_command_to_string
+ (cmd, from_tty, gdb_stdout->term_out ());
}
fi = get_selected_frame (_("frame apply "
"unable to get selected frame."));
@@ -1460,7 +1460,8 @@ thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
switch_to_thread (thr);
TRY
{
- std::string cmd_result = execute_command_to_string (cmd, from_tty);
+ std::string cmd_result = execute_command_to_string
+ (cmd, from_tty, gdb_stdout->term_out ());
if (!flags.silent || cmd_result.length () > 0)
{
if (!flags.quiet)
@@ -657,7 +657,8 @@ execute_command (const char *p, int from_tty)
temporarily set to true. */
std::string
-execute_command_to_string (const char *p, int from_tty)
+execute_command_to_string (const char *p, int from_tty,
+ bool term_out)
{
/* GDB_STDOUT should be better already restored during these
restoration callbacks. */
@@ -665,7 +666,7 @@ execute_command_to_string (const char *p, int from_tty)
scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0);
- string_file str_file;
+ string_file str_file (term_out);
{
current_uiout->redirect (&str_file);
@@ -101,6 +101,32 @@ ui_file_isatty (struct ui_file *file)
return file->isatty ();
}
+/* true if the gdb terminal supports styling, and styling is enabled. */
+static bool
+term_cli_styling (void)
+{
+ extern int cli_styling;
+
+ if (!cli_styling)
+ return false;
+
+ const char *term = getenv ("TERM");
+ /* Windows doesn't by default define $TERM, but can support styles
+ regardless. */
+#ifndef _WIN32
+ if (term == nullptr || !strcmp (term, "dumb"))
+ return false;
+#else
+ /* But if they do define $TERM, let us behave the same as on Posix
+ platforms, for the benefit of programs which invoke GDB as their
+ back-end. */
+ if (term && !strcmp (term, "dumb"))
+ return false;
+#endif
+ return true;
+}
+
+
void
ui_file_write (struct ui_file *file,
const char *buf,
@@ -131,6 +157,16 @@ fputs_unfiltered (const char *buf, struct ui_file *file)
+string_file::string_file ()
+{
+ m_term_out = false;
+}
+
+string_file::string_file (bool term_out)
+{
+ m_term_out = term_out;
+}
+
string_file::~string_file ()
{}
@@ -140,6 +176,18 @@ string_file::write (const char *buf, long length_buf)
m_string.append (buf, length_buf);
}
+bool
+string_file::term_out ()
+{
+ return m_term_out;
+}
+
+bool
+string_file::can_emit_style_escape ()
+{
+ return m_term_out && term_cli_styling ();
+}
+
stdio_file::stdio_file (FILE *file, bool close_p)
@@ -255,6 +303,14 @@ stdio_file::isatty ()
return ::isatty (m_fd);
}
+bool
+stdio_file::can_emit_style_escape ()
+{
+ return this == gdb_stdout
+ && this->isatty ()
+ && term_cli_styling ();
+}
+
/* This is the implementation of ui_file method 'write' for stderr.
@@ -332,3 +388,17 @@ tee_file::isatty ()
{
return m_one->isatty ();
}
+
+bool
+tee_file::term_out ()
+{
+ return m_one->term_out ();
+}
+
+bool
+tee_file::can_emit_style_escape ()
+{
+ return this == gdb_stdout
+ && m_one->term_out ()
+ && term_cli_styling ();
+}
@@ -70,6 +70,14 @@ public:
virtual bool isatty ()
{ return false; }
+ /* true indicates terminal output behaviour such as cli_styling. */
+ virtual bool term_out ()
+ { return isatty (); }
+
+ /* true if ANSI escapes can be used on STREAM. */
+ virtual bool can_emit_style_escape ()
+ { return false; }
+
virtual void flush ()
{}
};
@@ -109,7 +117,13 @@ extern int gdb_console_fputs (const char *, FILE *);
class string_file : public ui_file
{
public:
- string_file () {}
+ /* Construct a string_file to collect 'raw' output, i.e. without
+ 'terminal' behaviour such as cli_styling. */
+ string_file ();
+ /* If TERM_OUT, construct a string_file with terminal output behaviour
+ such as cli_styling)
+ else collect 'raw' output like the previous constructor. */
+ string_file (bool term_out);
~string_file () override;
/* Override ui_file methods. */
@@ -119,6 +133,9 @@ public:
long read (char *buf, long length_buf) override
{ gdb_assert_not_reached ("a string_file is not readable"); }
+ bool term_out () override;
+ bool can_emit_style_escape () override;
+
/* string_file-specific public API. */
/* Accesses the std::string containing the entire output collected
@@ -145,6 +162,8 @@ public:
private:
/* The internal buffer. */
std::string m_string;
+
+ bool m_term_out;
};
/* A ui_file implementation that maps directly onto <stdio.h>'s FILE.
@@ -183,6 +202,8 @@ public:
bool isatty () override;
+ bool can_emit_style_escape () override;
+
private:
/* Sets the internal stream to FILE, and saves the FILE's file
descriptor in M_FD. */
@@ -255,6 +276,8 @@ public:
void puts (const char *) override;
bool isatty () override;
+ bool term_out () override;
+ bool can_emit_style_escape () override;
void flush () override;
private:
@@ -1435,38 +1435,13 @@ emit_style_escape (const ui_file_style &style,
fputs_unfiltered (style.to_ansi ().c_str (), stream);
}
-/* See utils.h. */
-
-bool
-can_emit_style_escape (struct ui_file *stream)
-{
- if (stream != gdb_stdout
- || !cli_styling
- || !ui_file_isatty (stream))
- return false;
- const char *term = getenv ("TERM");
- /* Windows doesn't by default define $TERM, but can support styles
- regardless. */
-#ifndef _WIN32
- if (term == nullptr || !strcmp (term, "dumb"))
- return false;
-#else
- /* But if they do define $TERM, let us behave the same as on Posix
- platforms, for the benefit of programs which invoke GDB as their
- back-end. */
- if (term && !strcmp (term, "dumb"))
- return false;
-#endif
- return true;
-}
-
/* Set the current output style. This will affect future uses of the
_filtered output functions. */
static void
set_output_style (struct ui_file *stream, const ui_file_style &style)
{
- if (!can_emit_style_escape (stream))
+ if (!stream->can_emit_style_escape ())
return;
/* Note that we don't pass STREAM here, because we want to emit to
@@ -1479,7 +1454,7 @@ set_output_style (struct ui_file *stream, const ui_file_style &style)
void
reset_terminal_style (struct ui_file *stream)
{
- if (can_emit_style_escape (stream))
+ if (stream->can_emit_style_escape ())
{
/* Force the setting, regardless of what we think the setting
might already be. */
@@ -1504,7 +1479,7 @@ prompt_for_continue (void)
bool disable_pagination = pagination_disabled_for_command;
/* Clear the current styling. */
- if (can_emit_style_escape (gdb_stdout))
+ if (gdb_stdout->can_emit_style_escape ())
emit_style_escape (ui_file_style (), gdb_stdout);
if (annotation_level > 1)
@@ -1552,7 +1527,7 @@ prompt_for_continue (void)
pagination_disabled_for_command = disable_pagination;
/* Restore the current styling. */
- if (can_emit_style_escape (gdb_stdout))
+ if (gdb_stdout->can_emit_style_escape ())
emit_style_escape (applied_style);
dont_repeat (); /* Forget prev cmd -- CR won't repeat it. */
@@ -1805,7 +1780,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
lines_printed++;
if (wrap_column)
{
- if (can_emit_style_escape (stream))
+ if (stream->can_emit_style_escape ())
emit_style_escape (ui_file_style (), stream);
/* If we aren't actually wrapping, don't output
newline -- if chars_per_line is right, we
@@ -1827,7 +1802,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
if (wrap_column)
{
fputs_unfiltered (wrap_indent, stream);
- if (can_emit_style_escape (stream))
+ if (stream->can_emit_style_escape ())
emit_style_escape (wrap_style, stream);
/* FIXME, this strlen is what prevents wrap_indent from
containing tabs. However, if we recurse to print it
@@ -440,10 +440,6 @@ extern void fputs_styled (const char *linebuffer,
extern void reset_terminal_style (struct ui_file *stream);
-/* Return true if ANSI escapes can be used on STREAM. */
-
-extern bool can_emit_style_escape (struct ui_file *stream);
-
/* Display the host ADDR on STREAM formatted as ``0x%x''. */
extern void gdb_print_host_address_1 (const void *addr, struct ui_file *stream);