From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id URhCGvknAGDdIAAAWB0awg (envelope-from ) for ; Thu, 14 Jan 2021 06:16:09 -0500 Received: by simark.ca (Postfix, from userid 112) id 5E2971EF80; Thu, 14 Jan 2021 06:16:09 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 6A5141E590 for ; Thu, 14 Jan 2021 06:16:05 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AD59C3858002; Thu, 14 Jan 2021 11:16:04 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AD59C3858002 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1610622964; bh=CGP4BH9nWoAsxVBEoO04fkUfoXbxDmf0igej66xotJY=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=bHFhY2R6qJJu1QlGnQ2TdIpEUqd+Y08eahjIFexsqdnPsk5G1MxcgwRwyiP1AiWvZ NVsLTgBs8aiybBSNkc1zt+TYJTf0R0NsnLVbZPUADTeaHxjnSQXIMxaWZDtDbhUvvm s5UBBPO8aAGZq6D2VxPcQVu1vZYdYv0JVH3CJOwc= Received: from beryx.lancelotsix.com (beryx.lancelotsix.com [IPv6:2001:41d0:401:3000::1ab3]) by sourceware.org (Postfix) with ESMTPS id 4F8853858002 for ; Thu, 14 Jan 2021 11:16:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4F8853858002 Received: from Plymouth (unknown [IPv6:2a02:390:8443:0:18ff:4416:fd04:c7a6]) by beryx.lancelotsix.com (Postfix) with ESMTPSA id CDFE62E071; Thu, 14 Jan 2021 12:15:57 +0100 (CET) Date: Thu, 14 Jan 2021 11:15:54 +0000 To: Andrew Burgess Subject: Re: [PATCH 2/2] gdb: add new version style Message-ID: References: <04bf183a683d218b38a821a1108716191ea2cf22.1610618063.git.andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <04bf183a683d218b38a821a1108716191ea2cf22.1610618063.git.andrew.burgess@embecosm.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (beryx.lancelotsix.com [0.0.0.0]); Thu, 14 Jan 2021 12:15:58 +0100 (CET) X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Lancelot SIX via Gdb-patches Reply-To: Lancelot SIX Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Le Thu, Jan 14, 2021 at 09:56:43AM +0000, Andrew Burgess a écrit : > This commit adds a new 'version' style, which replaces the hard coded > styling currently used for GDB's version string. GDB's version number > is displayed: > > 1. In the output of 'show version', and > > 2. When GDB starts up (without the --quiet option). > > This new style can only ever affect the first of these two cases as > the second case is printed before GDB has processed any initialization > files, or processed any GDB commands passed on the command line. > > However, because the first case exists I think this commit makes > sense, it means the style is no longer hard coded into GDB, and we can > add some tests that the style can be enabled/disabled correctly. > > This commit is an alternative to a patch Tom posted here: > > https://sourceware.org/pipermail/gdb-patches/2020-June/169820.html > > I've used the style name 'version' instead of 'startup' to reflect > what the style is actually used for. If other parts of the startup > text end up being highlighted I imagine they would get their own > styles based on what is being highlighted. I feel this is more inline > with the other style names that are already in use within GDB. > > I also decoupled adding this style from the idea of startup options, > and the possibility of auto-saving startup options. Those ideas can > be explored in later patches. > > gdb/ChangeLog: > > * cli/cli-style.c: Add 'cli/cli-setshow.h' include. > (version_style): Define. > (cli_style_option::cli_style_option): Add intensity parameter, and > use as appropriate. > (_initialize_cli_style): Register version style set/show commands. > * cli/cli-style.h (cli_style_option): Add intensity parameter. > (version_style): Declare. > * top.c (print_gdb_version): Use version_stype, and styled_string > to print the GDB version string. > > gdb/doc/ChangeLog: > > * gdb.texinfo (Output Styling): Document version style. > > gdb/testsuite/ChangeLog: > > * gdb.base/style.exp (run_style_tests): Add version string test. > (test_startup_version_string): Use version style name. > * lib/gdb-utils.exp (style): Handle version style name. > --- > gdb/ChangeLog | 12 ++++++++++++ > gdb/NEWS | 5 +++++ > gdb/cli/cli-style.c | 17 +++++++++++++++-- > gdb/cli/cli-style.h | 6 +++++- > gdb/doc/ChangeLog | 4 ++++ > gdb/doc/gdb.texinfo | 12 ++++++++++++ > gdb/testsuite/ChangeLog | 6 ++++++ > gdb/testsuite/gdb.base/style.exp | 8 +++++++- > gdb/testsuite/lib/gdb-utils.exp | 1 + > gdb/top.c | 11 +++-------- > 10 files changed, 70 insertions(+), 12 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index 66702862efb..0ba0c0b5ea2 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -171,6 +171,11 @@ show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off). > executable file; if 'warn', just display a warning; if 'off', don't > attempt to detect a mismatch. > > +set style version foreground COLOR > +set style version background COLOR > +set style version intensity VALUE > + Control the styling of GDB's version number text. > + > tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]... > Define a new TUI layout, specifying its name and the windows that > will be displayed. > diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c > index cbedd30ea74..8b4b6b24cda 100644 > --- a/gdb/cli/cli-style.c > +++ b/gdb/cli/cli-style.c > @@ -19,6 +19,7 @@ > > #include "defs.h" > #include "cli/cli-cmds.h" > +#include "cli/cli-setshow.h" > #include "cli/cli-style.h" > #include "source-cache.h" > #include "observable.h" > @@ -98,13 +99,19 @@ cli_style_option metadata_style ("metadata", ui_file_style::DIM); > > /* See cli-style.h. */ > > +cli_style_option version_style ("version", ui_file_style::MAGENTA, > + ui_file_style::BOLD); > + > +/* See cli-style.h. */ > + > cli_style_option::cli_style_option (const char *name, > - ui_file_style::basic_color fg) > + ui_file_style::basic_color fg, > + ui_file_style::intensity intensity) > : changed (name), > m_name (name), > m_foreground (cli_colors[fg - ui_file_style::NONE]), > m_background (cli_colors[0]), > - m_intensity (cli_intensities[ui_file_style::NORMAL]) > + m_intensity (cli_intensities[intensity]) > { > } > > @@ -382,4 +389,10 @@ TUI window that does have the focus."), > &style_set_list, > &style_show_list, > true); > + > + version_style.add_setshow_commands (no_class, _("\ > +Version string display styling.\n\ > +Configure colors used to display the GDB version string."), > + &style_set_list, &style_show_list, > + false); > } > diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h > index cd51bf4aa94..187e1d07ce7 100644 > --- a/gdb/cli/cli-style.h > +++ b/gdb/cli/cli-style.h > @@ -30,7 +30,8 @@ class cli_style_option > public: > > /* Construct a CLI style option with a foreground color. */ > - cli_style_option (const char *name, ui_file_style::basic_color fg); > + cli_style_option (const char *name, ui_file_style::basic_color fg, > + ui_file_style::intensity = ui_file_style::NORMAL); > > /* Construct a CLI style option with an intensity. */ > cli_style_option (const char *name, ui_file_style::intensity i); > @@ -124,6 +125,9 @@ extern cli_style_option tui_border_style; > /* The border style of a TUI window that does have the focus. */ > extern cli_style_option tui_active_border_style; > > +/* The style to use for the GDB version string. */ > +extern cli_style_option version_style; > + > /* True if source styling is enabled. */ > extern bool source_styling; > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 69fa6b709bf..37f97bfb2b2 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -25872,6 +25872,18 @@ > @code{set style address} family of commands. By default, this style's > foreground color is blue. > > +@item version > +Control the styling of @value{GDBN}'s version number text. By > +default, this style's foreground color is magenta and it has bold > +intensity. The version number is displayed in two placed, the output s/placed/places/ > +of @command{show version}, and when @value{GDBN} starts up. > + > +Currently the version string displayed at startup is printed before > +@value{GDBN} has parsed any command line options, or parsed any > +command files, so there is currently no way to control the styling of > +this string. However, @value{GDBN}'s @code{--quiet} command line option > +can be used to disable printing of the version string on startup. > + > @item title > Control the styling of titles. These are managed with the > @code{set style title} family of commands. By default, this style's > diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp > index 58c8da0a10e..dfbadd6c1af 100644 > --- a/gdb/testsuite/gdb.base/style.exp > +++ b/gdb/testsuite/gdb.base/style.exp > @@ -264,6 +264,12 @@ proc run_style_tests { } { > gdb_test "p 5" \ > "Warning: '[style p title]', an alias for the command '[style print title]', is deprecated.*Use '[style new_p title]'.*" \ > "p deprecated warning, with replacement" > + > + # Check that the version string is styled in the output of 'show > + # version', and that this styling can be disabled. > + set vers [style "GNU gdb.*" version] > + gdb_test "show version" "${vers}.*" \ > + "version is styled in 'show version'" > } > } > > @@ -274,7 +280,7 @@ proc test_startup_version_string { } { > gdb_exit > gdb_spawn > > - set vers [style "GNU gdb.*" "35;1"] > + set vers [style "GNU gdb.*" version] > gdb_test "" "${vers}.*" "version is styled at startup" > } > > diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp > index 000e3800cd9..ad7d2884aae 100644 > --- a/gdb/testsuite/lib/gdb-utils.exp > +++ b/gdb/testsuite/lib/gdb-utils.exp > @@ -55,6 +55,7 @@ proc style {str style} { > variable { set style 36 } > address { set style 34 } > metadata { set style 2 } > + version { set style "35;1" } > } > return "\033\\\[${style}m${str}\033\\\[m" > } > diff --git a/gdb/top.c b/gdb/top.c > index 2c13864e120..92090bccbf4 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -1395,14 +1395,9 @@ print_gdb_version (struct ui_file *stream, bool interactive) > program to parse, and is just canonical program name and version > number, which starts after last space. */ > > - ui_file_style style; > - if (interactive) > - { > - ui_file_style nstyle = { ui_file_style::MAGENTA, ui_file_style::NONE, > - ui_file_style::BOLD }; > - style = nstyle; > - } > - fprintf_styled (stream, style, "GNU gdb %s%s\n", PKGVERSION, version); > + std::string v_str = string_printf ("GNU gdb %s%s", PKGVERSION, version); > + fprintf_filtered (stream, "%ps\n", > + styled_string (version_style.style (), v_str.c_str ())); > > /* Second line is a copyright notice. */ > > -- > 2.25.4 > It looks like this patch could be enough to closes the following ticket: https://sourceware.org/bugzilla/show_bug.cgi?id=25956 (with the limitation that this patch makes the version number configurable… after it has been displayed ; but at least the style is not hard-coded anymore). I tried to go through the remaining code and did not find other hard-coded styles (but I might have missed some). Lancelot.