Re: [PATCH 05/16] Add output styles to gdb

[Joel Brobecker <brobecker at adacore dot com>]

> gdb/ChangeLog
> 2018-11-27  Tom Tromey  <tom@tromey.com>
> 
> 	* utils.h (set_output_style, fprintf_styled)
> 	(fputs_styled): Declare.
> 	* utils.c (applied_style, desired_style): New globals.
> 	(emit_style_escape, set_output_style): New function.
> 	(prompt_for_continue): Emit style escapes.
> 	(fputs_maybe_filtered): Likewise.
> 	(fputs_styled, fprintf_styled): New functions.
> 	* ui-out.h (enum class ui_out_style_kind): New.
> 	(class ui_out) <field_string, field_stream, do_field_string>: Add
> 	style parameter.
> 	* ui-out.c (ui_out::field_stream, ui_out::field_string): Add style
> 	parameter.
> 	* tui/tui-out.h (class tui_ui_out) <do_field_string>: Add style
> 	parameter.
> 	* tui/tui-out.c (tui_ui_out::do_field_string): Add style
> 	parameter.
> 	(tui_ui_out::do_field_string): Update.
> 	* tracepoint.c (print_one_static_tracepoint_marker): Style
> 	output.
> 	* stack.c (print_frame_info, print_frame): Style output.
> 	* source.c (print_source_lines_base): Style output.
> 	* skip.c (info_skip_command): Style output.
> 	* record-btrace.c (btrace_call_history_src_line): Style output.
> 	(btrace_call_history): Likewise.
> 	* python/py-framefilter.c (py_print_frame): Style output.
> 	* mi/mi-out.h (class mi_ui_out) <do_field_string>: Add style
> 	parameter.
> 	* mi/mi-out.c (mi_ui_out::do_table_header)
> 	(mi_ui_out::do_field_int): Update.
> 	(mi_ui_out::do_field_string): Update.
> 	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn):
> 	Style output.
> 	* cli/cli-style.h: New file.
> 	* cli/cli-style.c: New file.
> 	* cli-out.h (class cli_ui_out) <do_field_string>: Add style
> 	parameter.
> 	* cli-out.c (cli_ui_out::do_table_header)
> 	(cli_ui_out::do_field_int, cli_ui_out::do_field_skip): Update.
> 	(cli_ui_out::do_field_string): Add style parameter.  Style the
> 	output.
> 	* breakpoint.c (print_breakpoint_location): Style output.
> 	(update_static_tracepoint): Likewise.
> 	* Makefile.in (SUBDIR_CLI_SRCS): Add cli-style.c.
> 	(HFILES_NO_SRCDIR): Add cli-style.h.
> 
> gdb/testsuite/ChangeLog
> 2018-11-27  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.base/style.exp: New file.
> 	* gdb.base/style.c: New file.

I tried to think about other ways to decide whether styling is
supported or not, but at the end of the day, Athis is just a detail,
and we can always come back to it. In the meantime, I think what
you've done should be good enough. I assume there aren't that
many terminals out there that don't support ansi colorizing.

The patch looks good to me overall. Just a few very minor nits.

> +void
> +cli_style_option::add_setshow_commands (const char *name,
> +					enum command_class theclass,
> +					const char *prefix_doc,
> +					const char *prefixname,
> +					struct cmd_list_element **set_list,
> +					struct cmd_list_element **show_list)
> +{
> +  m_show_prefix = std::string ("set ") + prefixname + " ";
> +  m_show_prefix = std::string ("show ") + prefixname + " ";
> +
> +  add_prefix_cmd (name, no_class, do_set, prefix_doc, &m_set_list, 

Small nit: Trailing space here...

> @@ -1182,7 +1185,7 @@ print_frame (struct frame_info *frame, int print_level,
>      string_file stb;
>      fprintf_symbol_filtered (&stb, funname ? funname.get () : "??",
>  			     funlang, DMGL_ANSI);
> -    uiout->field_stream ("func", stb);
> +    uiout->field_stream ("func", stb, ui_out_style_kind::FUNCTION);
>      uiout->wrap_hint ("   ");
>      annotate_frame_args ();
>        

Another one here as well.

> +++ b/gdb/testsuite/gdb.base/style.c
> @@ -0,0 +1,22 @@
> +/* Copyright 2018 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/>.  */
> +
> +#include <stdio.h>

Can we remove the #include in this case?

> +save_vars { env(TERM) } {
> +    # We need an ANSI-capable terminal to get the output.
> +    setenv TERM ansi
> +
> +    if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +	return -1
> +    }
> +
> +    if {![runto_main]} {
> +	fail "style tests failed"
> +	return
> +    }
> +
> +    gdb_test_no_output "set style enabled on"
> +    

Nit: A few trailing spaces here...


-- 
Joel