[RFA] Ensure GDB warnings are styled.
Commit Message
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 <philippe.waroquiers@skynet.be>
* 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 <philippe.waroquiers@skynet.be>
* 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(-)
Comments
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 <philippe.waroquiers@skynet.be>
>
> * 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 <philippe.waroquiers@skynet.be>
>
> * 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 ());
> }
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 <philippe.waroquiers@skynet.be>
>
> * 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 <philippe.waroquiers@skynet.be>
>
> * gdb.base/style.exp: Test that warnings are styled.
Thanks, that LGTM.
Simon
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 <philippe.waroquiers@skynet.be>
> >
> > * 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 <philippe.waroquiers@skynet.be>
> >
> > * gdb.base/style.exp: Test that warnings are styled.
>
> Thanks, that LGTM.
>
> Simon
>
@@ -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 ();
@@ -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]\"\\..*"
}
@@ -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
@@ -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 ());
}