From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 79681 invoked by alias); 3 Feb 2018 04:46:27 -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 79668 invoked by uid 89); 3 Feb 2018 04:46:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= 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, 03 Feb 2018 04:46:23 +0000 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id D07AE1E093; Fri, 2 Feb 2018 23:46:21 -0500 (EST) Subject: Re: [PATCH v3] Align natural-format register values to the same column To: Ruslan Kabatsayev , gdb-patches@sourceware.org References: <1516346545-8217-1-git-send-email-b7.10110111@gmail.com> From: Simon Marchi Message-ID: Date: Sat, 03 Feb 2018 04:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1516346545-8217-1-git-send-email-b7.10110111@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-02/txt/msg00045.txt.bz2 Hi Ruslan, The resulting output looks great. I have a few comments, some of them are identical to the review of v2. On 2018-01-19 02:22 AM, Ruslan Kabatsayev wrote: > Currently, commands such as "info reg", "info all-reg", as well as register > window in the TUI print badly aligned columns, like here: > > eax 0x1 1 > ecx 0xffffd3e0 -11296 > edx 0xffffd404 -11260 > ebx 0xf7fa5ff4 -134586380 > esp 0xffffd390 0xffffd390 > ebp 0xffffd3c8 0xffffd3c8 > esi 0x0 0 > edi 0x0 0 > eip 0x8048b60 0x8048b60 > eflags 0x286 [ PF SF IF ] > cs 0x23 35 > ss 0x2b 43 > ds 0x2b 43 > es 0x2b 43 > fs 0x0 0 > gs 0x63 99 > > After this patch, these commands print the third column values consistently > aligned one under another, provided the second column is not too long. > Originally, the third column was (attempted to be) aligned using a simple tab > character. This patch changes the alignment to spaces only. The tests checking > the output and expecting the single tab have been fixed in a previous patch, so > this change doesn't break any. > > gdb/ChangeLog: > > * infcmd.c (default_print_one_register_info): Align natural-format > column values consistently one under another. Please mention that pad_to_column was added. > --- > gdb/infcmd.c | 37 ++++++++++++++++++++++++++++--------- > 1 file changed, 28 insertions(+), 9 deletions(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 976276b..c59a8f8 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -2283,6 +2283,15 @@ path_command (const char *dirname, int from_tty) > } > > > +static void > +pad_to_column (string_file& stream, int col) Put the & after the space. > +{ > + stream.putc (' '); /* at least one space must be printed to separate columns */ Use a capital letter and period at the end (with two spaces after). Wrap at 80 columns. > + const int size = stream.size (); > + if (size < col) > + stream.puts (n_spaces (col - size)); > +} > + > /* Print out the register NAME with value VAL, to FILE, in the default > fashion. */ > > @@ -2293,9 +2302,17 @@ default_print_one_register_info (struct ui_file *file, > { > struct type *regtype = value_type (val); > int print_raw_format; > + string_file format_stream; > + enum tab_stops > + { > + value_column_1 = 15, > + /* Give enough room for "0x", 16 hex digits and two spaces in > + preceding column. */ > + value_column_2 = value_column_1+2+16+2, Reduce the indentation of these lines to 6 spaces, and add spaces around plus signs. > + }; > > - fputs_filtered (name, file); > - print_spaces_filtered (15 - strlen (name), file); > + format_stream.puts (name); > + format_stream.puts (n_spaces (value_column_1 - strlen (name))); Use pad_to_column? > > print_raw_format = (value_entirely_available (val) > && !value_optimized_out (val)); > @@ -2314,14 +2331,15 @@ default_print_one_register_info (struct ui_file *file, > > val_print (regtype, > value_embedded_offset (val), 0, > - file, 0, val, &opts, current_language); > + &format_stream, 0, val, &opts, current_language); > > if (print_raw_format) > { > - fprintf_filtered (file, "\t(raw "); > - print_hex_chars (file, valaddr, TYPE_LENGTH (regtype), byte_order, > + pad_to_column (format_stream, value_column_2); > + format_stream.puts ("(raw "); > + print_hex_chars (&format_stream, valaddr, TYPE_LENGTH (regtype), byte_order, This line is now too long. > true); > - fprintf_filtered (file, ")"); > + format_stream.puts (")"); You can use putc, since it's a single character. Thanks, Simon