[RFA] Ensure GDB warnings are styled.

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[RFA] Ensure GDB warnings are styled.

Philippe Waroquiers
While handling the comments of Tom related to
  [RFC] Have an option to tell GDB to detect and possibly handle mismatched exec-files.
  https://sourceware.org/ml/gdb-patches/2019-12/msg00621.html
I saw that GDB warnings are produced ignoring the given styles.

This patch:
  * ensures that style markups are properly handled by "warning".
  * changes 'set/show data-directory' so that file style is used
    in warnings and in 'show message'
  * changes all other messages in top.c to use file style when appropriate.
  * Uses the above data-directory changes in gdb.base/style.exp

2019-12-15  Philippe Waroquiers  <[hidden email]>

        * ui-file.c (stdio_file::can_emit_style_escape)
        (tee_file::can_emit_style_escape): Ensure style is used also on
        gdb_stderr when gdb_stderr is a tty supporting styling, similarly
        to gdb_stdout.
        * main.c (set_gdb_data_directory): Use file style to output the
        warning that the given pathname is not a directory.
        * top.c (show_history_filename, gdb_safe_append_history)
        (show_gdb_datadir): Use file style.

2019-12-15  Philippe Waroquiers  <[hidden email]>

        * gdb.base/style.exp: Test that warnings are styled.
---
 gdb/main.c                       |  3 ++-
 gdb/testsuite/gdb.base/style.exp |  6 ++++++
 gdb/top.c                        | 16 ++++++++++------
 gdb/ui-file.c                    |  4 ++--
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 1acf53ee92..f1ca8ce829 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -124,7 +124,8 @@ set_gdb_data_directory (const char *new_datadir)
       print_sys_errmsg (new_datadir, save_errno);
     }
   else if (!S_ISDIR (st.st_mode))
-    warning (_("%s is not a directory."), new_datadir);
+    warning (_("%ps is not a directory."),
+     styled_string (file_name_style.style (), new_datadir));
 
   gdb_datadir = gdb_realpath (new_datadir).get ();
 
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index c450f16b60..bb7f1005e8 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -139,4 +139,10 @@ save_vars { env(TERM) } {
 
     gdb_test "show logging file" \
  "The current logfile is \"[style .*? file]\"\\..*"
+
+    # Check warnings are styled by setting a rubbish data directory.
+    gdb_test "set data-directory Makefile" \
+ "warning: [style .*? file] is not a directory\\..*"
+    gdb_test "show data-directory" \
+ "GDB's data directory is \"[style .*? file]\"\\..*"
 }
diff --git a/gdb/top.c b/gdb/top.c
index bc300e4754..55efe8fbd6 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -55,6 +55,7 @@
 #include "gdbsupport/scope-exit.h"
 #include "gdbarch.h"
 #include "gdbsupport/pathstuff.h"
+#include "cli/cli-style.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -924,8 +925,8 @@ show_history_filename (struct ui_file *file, int from_tty,
        struct cmd_list_element *c, const char *value)
 {
   fprintf_filtered (file, _("The filename in which to record "
-    "the command history is \"%s\".\n"),
-    value);
+    "the command history is \"%ps\".\n"),
+    styled_string (file_name_style.style (), value));
 }
 
 /* This is like readline(), but it has some gdb-specific behavior.
@@ -1192,8 +1193,10 @@ gdb_safe_append_history (void)
   saved_errno = errno;
   if (ret < 0 && saved_errno != ENOENT)
     {
-      warning (_("Could not rename %s to %s: %s"),
-       history_filename, local_history_filename.c_str (),
+      warning (_("Could not rename %ps to %ps: %s"),
+       styled_string (file_name_style.style (), history_filename),
+       styled_string (file_name_style.style (),
+      local_history_filename.c_str ()),
        safe_strerror (saved_errno));
     }
   else
@@ -2067,8 +2070,9 @@ static void
 show_gdb_datadir (struct ui_file *file, int from_tty,
   struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file, _("GDB's data directory is \"%s\".\n"),
-    gdb_datadir.c_str ());
+  fprintf_filtered (file, _("GDB's data directory is \"%ps\".\n"),
+    styled_string (file_name_style.style (),
+   gdb_datadir.c_str ()));
 }
 
 static void
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 71b74bba19..728ee94d97 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -302,7 +302,7 @@ stdio_file::isatty ()
 bool
 stdio_file::can_emit_style_escape ()
 {
-  return (this == gdb_stdout
+  return ((this == gdb_stdout || this == gdb_stderr)
   && this->isatty ()
   && term_cli_styling ());
 }
@@ -391,7 +391,7 @@ tee_file::term_out ()
 bool
 tee_file::can_emit_style_escape ()
 {
-  return (this == gdb_stdout
+  return ((this == gdb_stdout || this == gdb_stderr)
   && m_one->term_out ()
   && term_cli_styling ());
 }
--
2.20.1

Reply | Threaded
Open this post in threaded view
|

PING. Re: [RFA] Ensure GDB warnings are styled.

Philippe Waroquiers
Ping.
Thanks
Philippe

On Sun, 2019-12-15 at 18:10 +0100, Philippe Waroquiers wrote:

> While handling the comments of Tom related to
>   [RFC] Have an option to tell GDB to detect and possibly handle mismatched exec-files.
>   https://sourceware.org/ml/gdb-patches/2019-12/msg00621.html
> I saw that GDB warnings are produced ignoring the given styles.
>
> This patch:
>   * ensures that style markups are properly handled by "warning".
>   * changes 'set/show data-directory' so that file style is used
>     in warnings and in 'show message'
>   * changes all other messages in top.c to use file style when appropriate.
>   * Uses the above data-directory changes in gdb.base/style.exp
>
> 2019-12-15  Philippe Waroquiers  <[hidden email]>
>
> * ui-file.c (stdio_file::can_emit_style_escape)
> (tee_file::can_emit_style_escape): Ensure style is used also on
> gdb_stderr when gdb_stderr is a tty supporting styling, similarly
> to gdb_stdout.
> * main.c (set_gdb_data_directory): Use file style to output the
> warning that the given pathname is not a directory.
> * top.c (show_history_filename, gdb_safe_append_history)
> (show_gdb_datadir): Use file style.
>
> 2019-12-15  Philippe Waroquiers  <[hidden email]>
>
> * gdb.base/style.exp: Test that warnings are styled.
> ---
>  gdb/main.c                       |  3 ++-
>  gdb/testsuite/gdb.base/style.exp |  6 ++++++
>  gdb/top.c                        | 16 ++++++++++------
>  gdb/ui-file.c                    |  4 ++--
>  4 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/main.c b/gdb/main.c
> index 1acf53ee92..f1ca8ce829 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -124,7 +124,8 @@ set_gdb_data_directory (const char *new_datadir)
>        print_sys_errmsg (new_datadir, save_errno);
>      }
>    else if (!S_ISDIR (st.st_mode))
> -    warning (_("%s is not a directory."), new_datadir);
> +    warning (_("%ps is not a directory."),
> +     styled_string (file_name_style.style (), new_datadir));
>  
>    gdb_datadir = gdb_realpath (new_datadir).get ();
>  
> diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
> index c450f16b60..bb7f1005e8 100644
> --- a/gdb/testsuite/gdb.base/style.exp
> +++ b/gdb/testsuite/gdb.base/style.exp
> @@ -139,4 +139,10 @@ save_vars { env(TERM) } {
>  
>      gdb_test "show logging file" \
>   "The current logfile is \"[style .*? file]\"\\..*"
> +
> +    # Check warnings are styled by setting a rubbish data directory.
> +    gdb_test "set data-directory Makefile" \
> + "warning: [style .*? file] is not a directory\\..*"
> +    gdb_test "show data-directory" \
> + "GDB's data directory is \"[style .*? file]\"\\..*"
>  }
> diff --git a/gdb/top.c b/gdb/top.c
> index bc300e4754..55efe8fbd6 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -55,6 +55,7 @@
>  #include "gdbsupport/scope-exit.h"
>  #include "gdbarch.h"
>  #include "gdbsupport/pathstuff.h"
> +#include "cli/cli-style.h"
>  
>  /* readline include files.  */
>  #include "readline/readline.h"
> @@ -924,8 +925,8 @@ show_history_filename (struct ui_file *file, int from_tty,
>         struct cmd_list_element *c, const char *value)
>  {
>    fprintf_filtered (file, _("The filename in which to record "
> -    "the command history is \"%s\".\n"),
> -    value);
> +    "the command history is \"%ps\".\n"),
> +    styled_string (file_name_style.style (), value));
>  }
>  
>  /* This is like readline(), but it has some gdb-specific behavior.
> @@ -1192,8 +1193,10 @@ gdb_safe_append_history (void)
>    saved_errno = errno;
>    if (ret < 0 && saved_errno != ENOENT)
>      {
> -      warning (_("Could not rename %s to %s: %s"),
> -       history_filename, local_history_filename.c_str (),
> +      warning (_("Could not rename %ps to %ps: %s"),
> +       styled_string (file_name_style.style (), history_filename),
> +       styled_string (file_name_style.style (),
> +      local_history_filename.c_str ()),
>         safe_strerror (saved_errno));
>      }
>    else
> @@ -2067,8 +2070,9 @@ static void
>  show_gdb_datadir (struct ui_file *file, int from_tty,
>    struct cmd_list_element *c, const char *value)
>  {
> -  fprintf_filtered (file, _("GDB's data directory is \"%s\".\n"),
> -    gdb_datadir.c_str ());
> +  fprintf_filtered (file, _("GDB's data directory is \"%ps\".\n"),
> +    styled_string (file_name_style.style (),
> +   gdb_datadir.c_str ()));
>  }
>  
>  static void
> diff --git a/gdb/ui-file.c b/gdb/ui-file.c
> index 71b74bba19..728ee94d97 100644
> --- a/gdb/ui-file.c
> +++ b/gdb/ui-file.c
> @@ -302,7 +302,7 @@ stdio_file::isatty ()
>  bool
>  stdio_file::can_emit_style_escape ()
>  {
> -  return (this == gdb_stdout
> +  return ((this == gdb_stdout || this == gdb_stderr)
>    && this->isatty ()
>    && term_cli_styling ());
>  }
> @@ -391,7 +391,7 @@ tee_file::term_out ()
>  bool
>  tee_file::can_emit_style_escape ()
>  {
> -  return (this == gdb_stdout
> +  return ((this == gdb_stdout || this == gdb_stderr)
>    && m_one->term_out ()
>    && term_cli_styling ());
>  }

Reply | Threaded
Open this post in threaded view
|

Re: [RFA] Ensure GDB warnings are styled.

Simon Marchi-4
In reply to this post by Philippe Waroquiers
On 2019-12-15 12:10 p.m., Philippe Waroquiers wrote:

> While handling the comments of Tom related to
>   [RFC] Have an option to tell GDB to detect and possibly handle mismatched exec-files.
>   https://sourceware.org/ml/gdb-patches/2019-12/msg00621.html
> I saw that GDB warnings are produced ignoring the given styles.
>
> This patch:
>   * ensures that style markups are properly handled by "warning".
>   * changes 'set/show data-directory' so that file style is used
>     in warnings and in 'show message'
>   * changes all other messages in top.c to use file style when appropriate.
>   * Uses the above data-directory changes in gdb.base/style.exp
>
> 2019-12-15  Philippe Waroquiers  <[hidden email]>
>
> * ui-file.c (stdio_file::can_emit_style_escape)
> (tee_file::can_emit_style_escape): Ensure style is used also on
> gdb_stderr when gdb_stderr is a tty supporting styling, similarly
> to gdb_stdout.
> * main.c (set_gdb_data_directory): Use file style to output the
> warning that the given pathname is not a directory.
> * top.c (show_history_filename, gdb_safe_append_history)
> (show_gdb_datadir): Use file style.
>
> 2019-12-15  Philippe Waroquiers  <[hidden email]>
>
> * gdb.base/style.exp: Test that warnings are styled.

Thanks, that LGTM.

Simon

Reply | Threaded
Open this post in threaded view
|

Re: [RFA] Ensure GDB warnings are styled.

Philippe Waroquiers
Thanks for the review, pushed.

Philippe


On Tue, 2019-12-31 at 13:13 -0500, Simon Marchi wrote:

> On 2019-12-15 12:10 p.m., Philippe Waroquiers wrote:
> > While handling the comments of Tom related to
> >   [RFC] Have an option to tell GDB to detect and possibly handle mismatched exec-files.
> >   https://sourceware.org/ml/gdb-patches/2019-12/msg00621.html
> > I saw that GDB warnings are produced ignoring the given styles.
> >
> > This patch:
> >   * ensures that style markups are properly handled by "warning".
> >   * changes 'set/show data-directory' so that file style is used
> >     in warnings and in 'show message'
> >   * changes all other messages in top.c to use file style when appropriate.
> >   * Uses the above data-directory changes in gdb.base/style.exp
> >
> > 2019-12-15  Philippe Waroquiers  <[hidden email]>
> >
> > * ui-file.c (stdio_file::can_emit_style_escape)
> > (tee_file::can_emit_style_escape): Ensure style is used also on
> > gdb_stderr when gdb_stderr is a tty supporting styling, similarly
> > to gdb_stdout.
> > * main.c (set_gdb_data_directory): Use file style to output the
> > warning that the given pathname is not a directory.
> > * top.c (show_history_filename, gdb_safe_append_history)
> > (show_gdb_datadir): Use file style.
> >
> > 2019-12-15  Philippe Waroquiers  <[hidden email]>
> >
> > * gdb.base/style.exp: Test that warnings are styled.
>
> Thanks, that LGTM.
>
> Simon
>