From: Tom Tromey Date: Tue, 9 Dec 2025 15:38:30 +0000 (-0700) Subject: Remove m_applied_style from ui_file X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0391f70c7d9d8cff7a2f651ba8b9b015d0e9ea5d;p=thirdparty%2Fbinutils-gdb.git Remove m_applied_style from ui_file While working on this series, I found a number of odd styling issues recurred. For instance, the issue where the pager would lose track and style subsequent output incorrectly reappeared. It turned out that different ui_file objects in the output pipeline would get confused about their current style. And, looking deeper at this, I realized that mainly it is the pager that really needs to track the current style at all. All the other file implementations can be purely reactive (except the buffered stream code, as Andrew pointed out). This patch moves m_applied_style from ui_file and into pager_file. This necessitated making ui_file::vprintf virtual, so that the base class could pass in the "plain" style as the starting point, whereas the pager could use the applied style. (I did not investigate whether this was truly necessary, and I somewhat suspect it might not be.) This straightforward approach caused some regressions, mostly involving extra ANSI escapes being emitted. I fixed most of these by arranging for ui_out::call_do_message to track styles a little more thoroughly. Co-Authored-By: Andrew Burgess Approved-By: Andrew Burgess --- diff --git a/gdb/buffered-streams.h b/gdb/buffered-streams.h index 9da45b08b6b..62598e70ca0 100644 --- a/gdb/buffered-streams.h +++ b/gdb/buffered-streams.h @@ -154,6 +154,15 @@ struct buffering_file : public ui_file return m_stream->can_emit_style_escape (); } + void emit_style_escape (const ui_file_style &style) override + { + if (can_emit_style_escape () && style != m_applied_style) + { + m_applied_style = style; + ui_file::emit_style_escape (style); + } + } + /* Flush the underlying output stream. */ void flush () override { @@ -167,6 +176,9 @@ private: /* The underlying output stream. */ ui_file *m_stream; + + /* The currently applied style. */ + ui_file_style m_applied_style; }; /* Attaches and detaches buffers for each of the gdb_std* streams. */ diff --git a/gdb/cli-out.c b/gdb/cli-out.c index b1ea9560fcf..d8ac13ab827 100644 --- a/gdb/cli-out.c +++ b/gdb/cli-out.c @@ -221,20 +221,24 @@ cli_ui_out::do_text (const char *string) } void -cli_ui_out::do_message (const ui_file_style &style, +cli_ui_out::do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, va_list args) { if (m_suppress_output) return; std::string str = string_vprintf (format, args); - if (!str.empty ()) + if (str.empty ()) + return; + + ui_file *stream = m_streams.back (); + if (current_style != style) { - ui_file *stream = m_streams.back (); stream->emit_style_escape (style); - stream->puts (str.c_str ()); - stream->emit_style_escape (ui_file_style ()); + current_style = style; } + stream->puts (str.c_str ()); } void @@ -489,6 +493,12 @@ cli_ui_out::can_emit_style_escape () const return m_streams.back ()->can_emit_style_escape (); } +void +cli_ui_out::emit_style_escape (const ui_file_style &style) +{ + m_streams.back ()->emit_style_escape (style); +} + /* CLI interface to display tab-completion matches. */ /* CLI version of displayer.crlf. */ diff --git a/gdb/cli-out.h b/gdb/cli-out.h index b34601f0f46..74307074a7c 100644 --- a/gdb/cli-out.h +++ b/gdb/cli-out.h @@ -35,6 +35,8 @@ public: bool can_emit_style_escape () const override; + void emit_style_escape (const ui_file_style &style) override; + ui_file *current_stream () const override { return m_streams.back (); } @@ -69,9 +71,10 @@ protected: override ATTRIBUTE_PRINTF (7, 0); virtual void do_spaces (int numspaces) override; virtual void do_text (const char *string) override; - virtual void do_message (const ui_file_style &style, + virtual void do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, va_list args) override - ATTRIBUTE_PRINTF (3,0); + ATTRIBUTE_PRINTF (4, 0); virtual void do_wrap_hint (int indent) override; virtual void do_flush () override; virtual void do_redirect (struct ui_file *outstream) override; diff --git a/gdb/mi/mi-out.c b/gdb/mi/mi-out.c index aac00ae7f76..41b0ee8cdbd 100644 --- a/gdb/mi/mi-out.c +++ b/gdb/mi/mi-out.c @@ -167,7 +167,8 @@ mi_ui_out::do_text (const char *string) } void -mi_ui_out::do_message (const ui_file_style &style, +mi_ui_out::do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, va_list args) { } diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h index ee70659abd3..d1d26718ac4 100644 --- a/gdb/mi/mi-out.h +++ b/gdb/mi/mi-out.h @@ -78,9 +78,10 @@ protected: override ATTRIBUTE_PRINTF (7,0); virtual void do_spaces (int numspaces) override; virtual void do_text (const char *string) override; - virtual void do_message (const ui_file_style &style, + virtual void do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, va_list args) override - ATTRIBUTE_PRINTF (3,0); + ATTRIBUTE_PRINTF (4, 0); virtual void do_wrap_hint (int indent) override; virtual void do_flush () override; virtual void do_redirect (struct ui_file *outstream) override; diff --git a/gdb/pager.h b/gdb/pager.h index 697134be805..114024d5e5f 100644 --- a/gdb/pager.h +++ b/gdb/pager.h @@ -51,6 +51,9 @@ public: m_stream->puts_unfiltered (str); } + void vprintf (const char *fmt, va_list args) override + ATTRIBUTE_PRINTF (2, 0); + private: void prompt_for_continue (); @@ -92,6 +95,9 @@ private: wrapping is not in effect. */ int m_wrap_column = 0; + /* The currently applied style. */ + ui_file_style m_applied_style; + /* The style applied at the time that wrap_here was called. */ ui_file_style m_wrap_style; diff --git a/gdb/python/py-uiout.h b/gdb/python/py-uiout.h index 159f1b22e46..21c068e9077 100644 --- a/gdb/python/py-uiout.h +++ b/gdb/python/py-uiout.h @@ -109,9 +109,10 @@ protected: void do_text (const char *string) override { } - void do_message (const ui_file_style &style, + void do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, va_list args) - override ATTRIBUTE_PRINTF (3,0) + override ATTRIBUTE_PRINTF (4, 0) { } void do_wrap_hint (int indent) override diff --git a/gdb/testsuite/gdb.python/py-styled-execute.exp b/gdb/testsuite/gdb.python/py-styled-execute.exp index afaaf928573..efc65d3be50 100644 --- a/gdb/testsuite/gdb.python/py-styled-execute.exp +++ b/gdb/testsuite/gdb.python/py-styled-execute.exp @@ -60,8 +60,7 @@ proc test_gdb_execute_styling {} { # Two possible outputs, BASIC_RE, the unstyled output text, or # STYLED_RE, the same things, but with styling applied. set text "\"version\" style" - set styled_text \ - [style "\"" version][style "version" version][style "\" style" version] + set styled_text [style $text version] set basic_re "The $text foreground color is: \[^\r\n\]+" set styled_re "The $styled_text foreground color is: \[^\r\n\]+" diff --git a/gdb/ui-file.c b/gdb/ui-file.c index c5931deaf20..d655e7edd9e 100644 --- a/gdb/ui-file.c +++ b/gdb/ui-file.c @@ -70,7 +70,7 @@ void ui_file::vprintf (const char *format, va_list args) { ui_out_flags flags = disallow_ui_out_field; - cli_ui_out (this, flags).vmessage (m_applied_style, format, args); + cli_ui_out (this, flags).vmessage ({}, format, args); } /* See ui-file.h. */ @@ -78,11 +78,8 @@ ui_file::vprintf (const char *format, va_list args) void ui_file::emit_style_escape (const ui_file_style &style) { - if (can_emit_style_escape () && style != m_applied_style) - { - m_applied_style = style; - this->puts (style.to_ansi ().c_str ()); - } + if (can_emit_style_escape ()) + this->puts (style.to_ansi ().c_str ()); } /* See ui-file.h. */ diff --git a/gdb/ui-file.h b/gdb/ui-file.h index 4aaf4d0e54e..84529de3618 100644 --- a/gdb/ui-file.h +++ b/gdb/ui-file.h @@ -55,7 +55,7 @@ public: void putc (int c); - void vprintf (const char *, va_list) ATTRIBUTE_PRINTF (2, 0); + virtual void vprintf (const char *, va_list) ATTRIBUTE_PRINTF (2, 0); /* Methods below are both public, and overridable by ui_file subclasses. */ @@ -139,11 +139,6 @@ public: this->puts (str); } -protected: - - /* The currently applied style. */ - ui_file_style m_applied_style; - private: /* Helper function for putstr and putstrn. Print the character C on diff --git a/gdb/ui-out.c b/gdb/ui-out.c index 8a41d9897fa..ac792b43905 100644 --- a/gdb/ui-out.c +++ b/gdb/ui-out.c @@ -559,7 +559,8 @@ ui_out::field_fmt (const char *fldname, const ui_file_style &style, } void -ui_out::call_do_message (const ui_file_style &style, const char *format, +ui_out::call_do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, ...) { va_list args; @@ -571,7 +572,7 @@ ui_out::call_do_message (const ui_file_style &style, const char *format, to put a "format" attribute on call_do_message. */ DIAGNOSTIC_PUSH DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL - do_message (style, format, args); + do_message (current_style, style, format, args); DIAGNOSTIC_POP va_end (args); @@ -583,6 +584,7 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, { format_pieces fpieces (&format, true); + ui_file_style current_style = in_style; ui_file_style style = in_style; for (auto &&piece : fpieces) @@ -608,13 +610,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, switch (piece.n_int_args) { case 0: - call_do_message (style, current_substring, str); + call_do_message (current_style, style, current_substring, + str); break; case 1: - call_do_message (style, current_substring, intvals[0], str); + call_do_message (current_style, style, current_substring, + intvals[0], str); break; case 2: - call_do_message (style, current_substring, + call_do_message (current_style, style, current_substring, intvals[0], intvals[1], str); break; } @@ -627,7 +631,8 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, gdb_assert_not_reached ("wide_char_arg not supported in vmessage"); break; case long_long_arg: - call_do_message (style, current_substring, va_arg (args, long long)); + call_do_message (current_style, style, current_substring, + va_arg (args, long long)); break; case int_arg: { @@ -635,13 +640,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, switch (piece.n_int_args) { case 0: - call_do_message (style, current_substring, val); + call_do_message (current_style, style, current_substring, + val); break; case 1: - call_do_message (style, current_substring, intvals[0], val); + call_do_message (current_style, style, current_substring, + intvals[0], val); break; case 2: - call_do_message (style, current_substring, + call_do_message (current_style, style, current_substring, intvals[0], intvals[1], val); break; } @@ -653,13 +660,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, switch (piece.n_int_args) { case 0: - call_do_message (style, current_substring, val); + call_do_message (current_style, style, current_substring, + val); break; case 1: - call_do_message (style, current_substring, intvals[0], val); + call_do_message (current_style, style, current_substring, + intvals[0], val); break; case 2: - call_do_message (style, current_substring, + call_do_message (current_style, style, current_substring, intvals[0], intvals[1], val); break; } @@ -671,13 +680,15 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, switch (piece.n_int_args) { case 0: - call_do_message (style, current_substring, val); + call_do_message (current_style, style, current_substring, + val); break; case 1: - call_do_message (style, current_substring, intvals[0], val); + call_do_message (current_style, style, current_substring, + intvals[0], val); break; case 2: - call_do_message (style, current_substring, + call_do_message (current_style, style, current_substring, intvals[0], intvals[1], val); break; } @@ -689,20 +700,23 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, switch (piece.n_int_args) { case 0: - call_do_message (style, current_substring, val); + call_do_message (current_style, style, current_substring, + val); break; case 1: - call_do_message (style, current_substring, intvals[0], val); + call_do_message (current_style, style, current_substring, + intvals[0], val); break; case 2: - call_do_message (style, current_substring, + call_do_message (current_style, style, current_substring, intvals[0], intvals[1], val); break; } } break; case double_arg: - call_do_message (style, current_substring, va_arg (args, double)); + call_do_message (current_style, style, current_substring, + va_arg (args, double)); break; case long_double_arg: gdb_assert_not_reached ("long_double_arg not supported in vmessage"); @@ -743,7 +757,7 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, case 's': { styled_string_s *ss = va_arg (args, styled_string_s *); - call_do_message (ss->style, "%s", ss->str); + call_do_message (current_style, ss->style, "%s", ss->str); } break; case '[': @@ -758,7 +772,8 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, } break; default: - call_do_message (style, current_substring, va_arg (args, void *)); + call_do_message (current_style, style, current_substring, + va_arg (args, void *)); break; } break; @@ -770,12 +785,16 @@ ui_out::vmessage (const ui_file_style &in_style, const char *format, because some platforms have modified GCC to include -Wformat-security by default, which will warn here if there is no argument. */ - call_do_message (style, current_substring, 0); + call_do_message (current_style, style, current_substring, 0); break; default: internal_error (_("failed internal consistency check")); } } + + ui_file_style plain; + if (can_emit_style_escape () && current_style != plain) + emit_style_escape (plain); } void diff --git a/gdb/ui-out.h b/gdb/ui-out.h index 69d9910443e..28af8d521e7 100644 --- a/gdb/ui-out.h +++ b/gdb/ui-out.h @@ -282,6 +282,11 @@ class ui_out escapes. */ virtual bool can_emit_style_escape () const = 0; + /* Emit a style escape, if possible. */ + virtual void emit_style_escape (const ui_file_style &style) + { + } + /* Return the ui_file currently used for output. */ virtual ui_file *current_stream () const = 0; @@ -361,9 +366,17 @@ protected: ATTRIBUTE_PRINTF (7, 0) = 0; virtual void do_spaces (int numspaces) = 0; virtual void do_text (const char *string) = 0; - virtual void do_message (const ui_file_style &style, + + /* A helper for vprintf and call_do_message. Formats a string and + then prints it using STYLE. This should take care to only change + the style when necessary (i.e., don't bother if the formatted + string is empty, or if the desired style is the same as + CURRENT_STYLE). Updates current_style if the style was + changed. */ + virtual void do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, va_list args) - ATTRIBUTE_PRINTF (3,0) = 0; + ATTRIBUTE_PRINTF (4, 0) = 0; virtual void do_wrap_hint (int indent) = 0; virtual void do_flush () = 0; virtual void do_redirect (struct ui_file *outstream) = 0; @@ -380,7 +393,10 @@ protected: { return false; } private: - void call_do_message (const ui_file_style &style, const char *format, + /* A helper for vmessage that wraps a call to do_message. This will + update CURRENT_STYLE when needed. */ + void call_do_message (ui_file_style ¤t_style, + const ui_file_style &style, const char *format, ...); ui_out_flags m_flags; diff --git a/gdb/utils.c b/gdb/utils.c index 2f8bf5551c5..061161b1f3a 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1673,6 +1673,13 @@ pager_file::check_for_overfull_line (const unsigned int lines_allowed) } } +void +pager_file::vprintf (const char *format, va_list args) +{ + ui_out_flags flags = disallow_ui_out_field; + cli_ui_out (this, flags).vmessage (m_applied_style, format, args); +} + void pager_file::puts (const char *linebuffer) {