[RFC,5/6] Let the user control the startup style

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

Commit Message

Tom Tromey April 4, 2020, 2:54 p.m.
Some users find the startup text highlighting to be distracting.  This
patch provides a way to change this.  In particular, this adds a new
file to let the user control the styling of the startup text.  When
these styles are changed, they are written to this config file.  The
config file in turn is ready early in startup, so that they can take
effect before the welcome message is printed.

gdb/ChangeLog
2020-03-28  Tom Tromey  <tom@tromey.com>

	* NEWS: Add entry.
	* top.c (startup_style): Remove.
	(print_tip, print_gdb_version): Update.
	* main.c (STARTUP_STYLE_FILE): New define.
	(write_startup_style, read_startup_style): New functions.
	(captured_main_1): Call read_startup_style.
	* 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.

gdb/doc/ChangeLog
2020-03-28  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Output Styling): Document "set style startup"
	commands.
---
 gdb/ChangeLog       | 17 ++++++++++++++++
 gdb/NEWS            |  7 +++++++
 gdb/cli/cli-style.c | 49 +++++++++++++++++++++++++++++++++++++++++++--
 gdb/cli/cli-style.h | 11 +++++++++-
 gdb/doc/ChangeLog   |  5 +++++
 gdb/doc/gdb.texinfo | 15 ++++++++++++++
 gdb/main.c          | 43 +++++++++++++++++++++++++++++++++++++++
 gdb/top.c           | 12 ++---------
 8 files changed, 146 insertions(+), 13 deletions(-)

-- 
2.17.2

Comments

Eli Zaretskii April 4, 2020, 4:21 p.m. | #1
> From: Tom Tromey <tom@tromey.com>

> Date: Sat,  4 Apr 2020 08:54:21 -0600

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

> 

> index b376737e409..bb54c738281 100644

> --- a/gdb/NEWS

> +++ b/gdb/NEWS

> @@ -60,6 +60,13 @@ show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).

>    whether to load the process 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 part is OK.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo

> index 385c832f222..f860b238ff8 100644

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

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

> @@ -25573,6 +25573,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.


This is also OK.  But I wonder: we have ~/.gdbinit and ~/config/gdb
(or, if XDG_CONFIG_HOME is set, some utterly different directory) for
the new file?  Sounds confusing.  Also, I'm told that XDG_CONFIG_HOME
is ephemeral: it is set anew each session, so the user could end up
having his/her setting saved in a file he/she will not see the next
time they log in.  Can that happen?

Thanks.
Tom Tromey June 21, 2020, 8:27 p.m. | #2
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:


>> +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.


Eli> This is also OK.  But I wonder: we have ~/.gdbinit and ~/config/gdb
Eli> (or, if XDG_CONFIG_HOME is set, some utterly different directory) for
Eli> the new file?  Sounds confusing.  Also, I'm told that XDG_CONFIG_HOME
Eli> is ephemeral: it is set anew each session, so the user could end up
Eli> having his/her setting saved in a file he/she will not see the next
Eli> time they log in.  Can that happen?

I think XDG_CONFIG_HOME is intended for user config files written by the
app and as such isn't reset each session.  See

https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

The XDG "cache" and "runtime" directories can be reset, on the other
hand.

Tom

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index b376737e409..bb54c738281 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -60,6 +60,13 @@  show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).
   whether to load the process 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 d2d9928acd5..c9576fa2534 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -22,6 +22,7 @@ 
 #include "cli/cli-style.h"
 #include "source-cache.h"
 #include "observable.h"
+#include "gdbsupport/pathstuff.h"
 
 /* True if styling is enabled.  */
 
@@ -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])
 {
 }
 
@@ -257,6 +264,39 @@  cli_style_option::add_setshow_commands (enum command_class theclass,
 			  &m_set_list, &m_show_list, (void *) this);
 }
 
+bool
+cli_style_option::write (const char *filename)
+{
+  std::string config_dir = get_standard_config_dir ();
+
+  if (config_dir.empty ())
+    {
+      warning (_("Couldn't determine a path for the startup style settings."));
+      return false;
+    }
+
+  if (!mkdir_recursive (config_dir.c_str ()))
+    {
+      warning (_("Could not make config directory: %s"),
+	       safe_strerror (errno));
+      return false;
+    }
+
+  std::string fullname = config_dir + "/" + filename;
+  stdio_file outfile;
+
+  if (!outfile.open (fullname.c_str (), FOPEN_WT))
+    perror_with_name (fullname.c_str ());
+  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);
+
+  return true;
+}
+
 static cmd_list_element *style_set_list;
 static cmd_list_element *style_show_list;
 
@@ -422,4 +462,9 @@  TUI active border display styling.\n\
 Configure TUI active border colors\n\
 The \"tui-active-border\" style is used when GDB displays the border of a\n\
 TUI window that does have the focus."), true);
+
+  STYLE_ADD_SETSHOW_COMMANDS (startup_style,
+			      _("\
+Startup display styling.\n\
+Configure colors used in some startup text."), false);
 }
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 04009aa3615..28c3c8b2dde 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);
@@ -58,6 +59,11 @@  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 FILENAME, relative to the config directory.
+     (This is only needed for the startup style.)  Returns true on
+     success and false on failure.  */
+  bool write (const char *filename);
+
   /* This style can be observed for any changes.  */
   gdb::observers::observable<> changed;
 
@@ -126,6 +132,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 385c832f222..f860b238ff8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -25573,6 +25573,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/main.c b/gdb/main.c
index a03ed8117ab..51e422ae496 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -437,6 +437,45 @@  validate_readnow_readnever ()
     }
 }
 
+/* File name for startup style.  */
+
+#define STARTUP_STYLE_FILE "startup-style"
+
+/* Write the startup style to a file.  */
+
+static void
+write_startup_style ()
+{
+  static bool still_ok = true;
+
+  if (still_ok)
+    still_ok = startup_style.write (STARTUP_STYLE_FILE);
+}
+
+/* Read the startup style and arrange to track it.  */
+
+static void
+read_startup_style ()
+{
+  std::string config_dir = get_standard_config_dir ();
+
+  if (!config_dir.empty ())
+    {
+      std::string filename = config_dir + "/" + STARTUP_STYLE_FILE;
+
+      try
+	{
+	  source_script (filename.c_str (), 1);
+	}
+      catch (const gdb_exception &)
+	{
+	  /* Ignore errors.  */
+	}
+
+      startup_style.changed.attach (write_startup_style);
+    }
+}
+
 /* Type of this option.  */
 enum cmdarg_kind
 {
@@ -877,6 +916,10 @@  captured_main_1 (struct captured_main_args *context)
   /* Initialize all files.  */
   gdb_init (gdb_program_name);
 
+  /* Set the startup style.  */
+  if (!inhibit_gdbinit && !inhibit_home_gdbinit)
+    read_startup_style ();
+
   /* Now that gdb_init has created the initial inferior, we're in
      position to set args for that inferior.  */
   if (set_args)
diff --git a/gdb/top.c b/gdb/top.c
index 7af820b0bb9..46781a5c0ef 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.  */
 
@@ -1434,7 +1426,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 ());
 }
@@ -1449,7 +1441,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.  */