From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125493 invoked by alias); 6 Oct 2018 15:53:58 -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 125469 invoked by uid 89); 6 Oct 2018 15:53:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=bold, layers, NOTHING, seemed X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 06 Oct 2018 15:53:54 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 9FC3F1E4C2; Sat, 6 Oct 2018 11:53:52 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=simark.ca; s=mail; t=1538841232; bh=AMUveTIO3aXYvlFQkWUvsLcGJxoB9MoKoXyPfJE5Tu4=; h=Subject:To:References:From:Date:In-Reply-To:From; b=mv1P5uKIvGgTvp8/AdhOK0UlequwuIo95jlx1i+j3GwqFEuO82lA4ECnPwYzzVHo/ CpEDZqKP+YWja3OoXEG1lzNKhVNgPTbDITUblqbLxb7bmulDQXgGQUSuBDe5NOC0d4 +vCm5VBCd7tjubEsf2wBwufChxailVD2UnKq5bE0= Subject: Re: [RFC 3/8] Add output styles to gdb To: Tom Tromey , gdb-patches@sourceware.org References: <20180906211303.11029-1-tom@tromey.com> <20180906211303.11029-4-tom@tromey.com> From: Simon Marchi Message-ID: Date: Sat, 06 Oct 2018 15:53:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <20180906211303.11029-4-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-10/txt/msg00146.txt.bz2 On 2018-09-06 5:12 p.m., Tom Tromey wrote: > This adds some output styling to the CLI. > > A style is currently a foreground color, a background color, and an > intensity (dim or bold). (This list could be expanded depending on > terminal capabilities.) > > A style can be applied while printing. This patch changes cli-out.c > to apply styles just to certain fields, recognized by name. In > particular, function names and file names are stylized. > > This seemed like a reasonable approach, because the names are fixed > due to their use in MI. That is, the CLI is just as exposed to future > changes as MI is. > > Users can control the style via a number of new set/show commands. In > the interest of not typing many nearly-identical documentation > strings, I automated this. On the down side, this is not very > i18n-friendly. > > I've chose some default colors to use. I think it would be good to > enable this by default, so that when users start the new gdb, they > will see the new feature. > > Stylizing is done if TERM is set and is not "dumb". This could be > improved when the TUI is available by using the curses has_colors > call. That is, the lowest layer could call this without committing to > using curses everywhere; see my other patch for TUI colorizing. > > I considered adding a new "set_style" method to ui_file. However, > because the implementation had to interact with the pager code, I > didn't take this approach. But, one idea might be to put the isatty > check there and then have it defer to the lower layers. Hi Tom, Overall this looks great. I'm not too worried about internationalization. I think in this case for example: _("The \"%s\" display intensity is: %s\n"), name, value If "name" corresponds to a field name or sub-command name (like "filename"), it's probably better to leave it in english. If it refers to the concept of a filename, then we would want to translate it. In the latter case, I guess we could do it in two steps, and also pass the value through gettext: std::string tmpl = string_printf ("The %s display intensity is: %%s\n", name); printf (_(tmpl.c_str ()), _(value)); So tmpl would contain "The filename display intensity is: %s\n", which could be looked up by gettext to something that has the proper translation for "filename". Then, the color name would be translated too. Or maybe I don't understand how gettext works. I'm just not sure about choosing styles using the field name. For a filename, you could have a bunch of different field names, file filename, original_filename, absolute_filename, etc. So it can quickly become a bit crowded here. Could we pass an additional enum parameter to do_field_string to indicate the type of element this field represents? If this parameter has a default value of "NOTHING", then we can add then incrementally. Or it can even be an hybrid approach, where we try to match field names, which works 95% of the time, and then have this enum parameter to override the auto-detection if needed. Simon