From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88483 invoked by alias); 24 Dec 2018 04:08:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 87551 invoked by uid 89); 24 Dec 2018 04:08:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=styles, Declare, Style X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 24 Dec 2018 04:07:59 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 87DC7117125; Sun, 23 Dec 2018 23:07:57 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 1F+RkU6134Tm; Sun, 23 Dec 2018 23:07:57 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id BB4E611711E; Sun, 23 Dec 2018 23:07:55 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id D611C86756; Mon, 24 Dec 2018 08:07:48 +0400 (+04) Date: Mon, 24 Dec 2018 04:08:00 -0000 From: Joel Brobecker To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 05/16] Add output styles to gdb Message-ID: <20181224040748.GH5246@adacore.com> References: <20181128001435.12703-1-tom@tromey.com> <20181128001435.12703-6-tom@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181128001435.12703-6-tom@tromey.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-SW-Source: 2018-12/txt/msg00292.txt.bz2 > gdb/ChangeLog > 2018-11-27 Tom Tromey > > * 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) : 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) : 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) : 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) : 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 > > * 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 . */ > + > +#include 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