[v2,6/7] Let the user control the startup style

Message ID 20200623132006.15863-7-tom@tromey.com
State New
Headers show
Series
  • Some user-friendliness changes
Related show

Commit Message

Tom Tromey June 23, 2020, 1:20 p.m.
Some users find the startup text highlighting to be distracting.  This
patch provides a way to change this, building on the startup file
infrastructure that was added earlier.

gdb/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* tips: Add a tip.
	* NEWS: Add entry.
	* top.c (startup_style): Remove.
	(print_tip, print_gdb_version): Update.
	* cli/cli-style.h (class cli_style_option) <cli_style_option>: Add
	intensity parameter.
	<write>: Declare new method.
	(startup_style): Declare.
	* cli/cli-style.c (startup_style): New global.
	(cli_style_option): Add intensity parameter.
	(cli_style_option::write): New method.
	(_initialize_cli_style): Register new style and callbacks.

gdb/doc/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Output Styling): Document "set style startup"
	commands.

gdb/testsuite/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* gdb.base/persist.exp: New file.
---
 gdb/ChangeLog                      | 15 +++++++
 gdb/NEWS                           |  7 ++++
 gdb/cli/cli-style.c                | 36 ++++++++++++++++-
 gdb/cli/cli-style.h                |  9 ++++-
 gdb/doc/ChangeLog                  |  5 +++
 gdb/doc/gdb.texinfo                | 15 +++++++
 gdb/testsuite/ChangeLog            |  4 ++
 gdb/testsuite/gdb.base/persist.exp | 64 ++++++++++++++++++++++++++++++
 gdb/tips                           |  3 ++
 gdb/top.c                          | 12 +-----
 10 files changed, 157 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/persist.exp

-- 
2.17.2

Comments

Eli Zaretskii June 23, 2020, 2:41 p.m. | #1
> From: Tom Tromey <tom@tromey.com>

> Date: Tue, 23 Jun 2020 07:20:05 -0600

> Cc: Tom Tromey <tom@tromey.com>

> 

> diff --git a/gdb/NEWS b/gdb/NEWS

> index e977630c445..8f474c51bf4 100644

> --- a/gdb/NEWS

> +++ b/gdb/NEWS

> @@ -80,6 +80,13 @@ 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 startup foreground COLOR

> +set style startup background COLOR

> +set style startup intensity VALUE

> +  Control the styling of startup text.  This saves the setting into

> +  a special configuration file, so that it can be read during startup

> +  and applied.

> +


This is OK.

> --- a/gdb/doc/gdb.texinfo

> +++ b/gdb/doc/gdb.texinfo

> @@ -25672,6 +25672,21 @@ Control the styling of addresses.  These are managed with the

>  @code{set style address} family of commands.  By default, this style's

>  foreground color is blue.

>  

> +@item startup

> +Control the styling of some text that is printed at startup.  These

> +are managed with the @code{set style startup} family of commands.  By

> +default, this style's foreground color is magenta and it has bold

> +intensity.  Changing these settings will cause them to automatically

> +be saved in a special configuration file, which is read by

> +@value{GDBN} early in its startup.

> +

> +The default value for this directory depends on the host platform.  On

                         ^^^^^^^^^^^^^^
The word "directory" doesn't appear in the previous paragraph, so it
is unclear what "this directory" refers to.

> +most systems, the index is cached in the @file{gdb} subdirectory of

                 ^^^^^^^^^^^^^^^^^^^
Likewise with "index".

The documentation parts are OK with those nits fixed.  Thanks.
Tom Tromey July 5, 2020, 6:50 p.m. | #2
>> +@item startup

>> +Control the styling of some text that is printed at startup.  These

>> +are managed with the @code{set style startup} family of commands.  By

>> +default, this style's foreground color is magenta and it has bold

>> +intensity.  Changing these settings will cause them to automatically

>> +be saved in a special configuration file, which is read by

>> +@value{GDBN} early in its startup.

>> +

>> +The default value for this directory depends on the host platform.  On

Eli>                          ^^^^^^^^^^^^^^
Eli> The word "directory" doesn't appear in the previous paragraph, so it
Eli> is unclear what "this directory" refers to.

>> +most systems, the index is cached in the @file{gdb} subdirectory of

Eli>                  ^^^^^^^^^^^^^^^^^^^
Eli> Likewise with "index".

Eli> The documentation parts are OK with those nits fixed.  Thanks.

Thanks for the review.  I copied some text from the index cache section,
but I didn't sufficiently edit it.  This appeared in another patch in
this series as well.

In both spots now, I've changed this paragraph to read:

+The directory in which this file appears depends on the host platform.
+On most systems, the file is in the @file{gdb} subdirectory of the
+directory pointed to by the @env{XDG_CONFIG_HOME} environment
+variable, if it is defined, else in the @file{.config/gdb}
+subdirectory of your home directory.  However, on some systems, the
+default may differ according to local convention.

Tom
Eli Zaretskii July 5, 2020, 7:02 p.m. | #3
> From: Tom Tromey <tom@tromey.com>

> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org

> Date: Sun, 05 Jul 2020 12:50:23 -0600

> 

> +The directory in which this file appears depends on the host platform.

> +On most systems, the file is in the @file{gdb} subdirectory of the

> +directory pointed to by the @env{XDG_CONFIG_HOME} environment

> +variable, if it is defined, else in the @file{.config/gdb}

> +subdirectory of your home directory.  However, on some systems, the

> +default may differ according to local convention.


LGTM, thanks.

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index e977630c445..8f474c51bf4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -80,6 +80,13 @@  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 startup foreground COLOR
+set style startup background COLOR
+set style startup intensity VALUE
+  Control the styling of startup text.  This saves the setting into
+  a special configuration file, so that it can be read during startup
+  and applied.
+
 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 a0c3cc51801..8486d22ecd9 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 startup_style ("startup", 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])
 {
 }
 
@@ -253,6 +260,17 @@  cli_style_option::add_setshow_commands (enum command_class theclass,
 			  &m_set_list, &m_show_list, (void *) this);
 }
 
+void
+cli_style_option::write (ui_file *outfile)
+{
+  fprintf_unfiltered (outfile, "set style %s background %s\n",
+		      m_name, m_background);
+  fprintf_unfiltered (outfile, "set style %s foreground %s\n",
+		      m_name, m_foreground);
+  fprintf_unfiltered (outfile, "set style %s intensity %s\n",
+		      m_name, m_intensity);
+}
+
 static cmd_list_element *style_set_list;
 static cmd_list_element *style_show_list;
 
@@ -382,4 +400,18 @@  TUI window that does have the focus."),
 						&style_set_list,
 						&style_show_list,
 						true);
+
+  startup_style.add_setshow_commands (no_class, _("\
+Startup display styling.\n\
+Configure colors used in some startup text."),
+				      &style_set_list, &style_show_list,
+				      false);
+  /* Ensure that the startup style is written to the startup file.  */
+  add_startup_writer ([] (ui_file *outfile)
+    {
+      startup_style.write (outfile);
+    });
+  /* Arrange to write the startup file whenever a startup style
+     setting changes.  */
+  startup_style.changed.attach (write_startup_file);
 }
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 6422e5296a3..28859f88560 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);
@@ -56,6 +57,9 @@  class cli_style_option
   /* Same as SET_LIST but for the show command list.  */
   struct cmd_list_element *show_list () { return m_show_list; };
 
+  /* Write this style to FILE.  */
+  void write (ui_file *outfile);
+
   /* This style can be observed for any changes.  */
   gdb::observers::observable<> changed;
 
@@ -124,6 +128,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 (some) startup text.  */
+extern cli_style_option startup_style;
+
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0a74992fbac..7c3fe7d38a2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -25672,6 +25672,21 @@  Control the styling of addresses.  These are managed with the
 @code{set style address} family of commands.  By default, this style's
 foreground color is blue.
 
+@item startup
+Control the styling of some text that is printed at startup.  These
+are managed with the @code{set style startup} family of commands.  By
+default, this style's foreground color is magenta and it has bold
+intensity.  Changing these settings will cause them to automatically
+be saved in a special configuration file, which is read by
+@value{GDBN} early in its startup.
+
+The default value for this directory depends on the host platform.  On
+most systems, the index is cached in the @file{gdb} subdirectory of
+the directory pointed to by the @env{XDG_CONFIG_HOME} environment
+variable, if it is defined, else in the @file{.config/gdb} subdirectory
+of your home directory.  However, on some systems, the default may
+differ according to local convention.
+
 @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/persist.exp b/gdb/testsuite/gdb.base/persist.exp
new file mode 100644
index 00000000000..76a55d558c3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/persist.exp
@@ -0,0 +1,64 @@ 
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# This test relies on XDG_CONFIG_HOME, which does not work on all
+# platforms; so limit it to Linux native.
+if {![istarget "*-*-linux*"]} then {
+    continue
+}
+if { [target_info gdb_protocol] != "" } {
+    continue
+}
+
+# Check that the contents of FILENAME changed and that the contents
+# now contain the given command.  CONTENTS is the old contents.
+# COMMAND is the command to run, and also used to name the test.
+# Returns the new contents.
+proc require_changed {command filename contents} {
+    gdb_test_no_output $command
+
+    set fd [open $filename r]
+    set new [read $fd]
+    close $fd
+
+    set testname "$command check"
+    if {$contents == $new || [string first $command $new] == -1} {
+	fail $testname
+    } else {
+	pass $testname
+    }
+
+    return $new
+}
+
+save_vars { env(XDG_CONFIG_HOME) } {
+    set dirname [standard_output_file .]
+    file mkdir $dirname/gdb
+    set filename $dirname/gdb/startup-commands
+
+    # Create the file as empty.
+    set fd [open $filename w]
+    close $fd
+
+    # Current contents.
+    set contents ""
+
+    setenv XDG_CONFIG_HOME $dirname
+    clean_restart
+
+    set contents [require_changed "set style startup foreground green" $filename $contents]
+    set contents [require_changed "set style startup background green" $filename $contents]
+    set contents [require_changed "set style startup intensity dim" $filename $contents]
+}
diff --git a/gdb/tips b/gdb/tips
index 75957fb853d..a9c89747aa4 100644
--- a/gdb/tips
+++ b/gdb/tips
@@ -7,3 +7,6 @@  You can use "help news" to see what has changed in GDB.
 Type "apropos word" to search for commands related to "word".
 %
 Type "show configuration" for configuration details.
+%
+You can use the "set style startup" family of commands to change the
+style used for tips.
diff --git a/gdb/top.c b/gdb/top.c
index 62a2c706031..e86ce4020f3 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -130,14 +130,6 @@  current_ui_current_uiout_ptr ()
 
 int inhibit_gdbinit = 0;
 
-/* The style used for informational messages at startup.  */
-static ui_file_style startup_style =
-{
-  ui_file_style::MAGENTA,
-  ui_file_style::NONE,
-  ui_file_style::BOLD
-};
-
 /* Flag for whether we want to confirm potentially dangerous
    operations.  Default is yes.  */
 
@@ -1447,7 +1439,7 @@  print_tip (struct ui_file *stream)
   std::uniform_int_distribution<> distr (0, strings.size () - 1);
   int index = distr (mt);
   /* Note that the string will include a newline.  */
-  fprintf_styled (stream, startup_style, "\n%.*s",
+  fprintf_styled (stream, startup_style.style (), "\n%.*s",
 		  (int) strings[index].size (),
 		  strings[index].data ());
 }
@@ -1462,7 +1454,7 @@  print_gdb_version (struct ui_file *stream, bool interactive)
 
   ui_file_style style;
   if (interactive)
-    style = startup_style;
+    style = startup_style.style ();
   fprintf_styled (stream, style, "GNU gdb %s%s\n", PKGVERSION, version);
 
   /* Second line is a copyright notice.  */