From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9917 invoked by alias); 26 Jul 2017 09:33:26 -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 9282 invoked by uid 89); 26 Jul 2017 09:33:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:4392 X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Jul 2017 09:33:24 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v6Q9XHx4012917 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 26 Jul 2017 05:33:22 -0400 Received: by simark.ca (Postfix, from userid 112) id 406F41E982; Wed, 26 Jul 2017 05:33:17 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 6EE371E891; Wed, 26 Jul 2017 05:33:15 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 26 Jul 2017 09:33:00 -0000 From: Simon Marchi To: Ruslan Kabatsayev Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2] Align natural-format register values to the same column In-Reply-To: <1501055625-25029-1-git-send-email-b7.10110111@gmail.com> References: <1501055625-25029-1-git-send-email-b7.10110111@gmail.com> Message-ID: <5bce7397bf829abb601f61f6e40877c8@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 26 Jul 2017 09:33:17 +0000 X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00408.txt.bz2 Hi Ruslan, Thanks for the update, I just have some minor comments, but otherwise the patch looks good to me. On 2017-07-26 09:53, 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. Lots of tests in the test suite check for it, so this patch > retains > the tab in the output after the second column. This allows these tests > to > continue working unchanged. What is different is that now the tab may > be > followed by several spaces, which complete the task of aligning the > third > column when the sole tab doesn't work well. > > gdb/ChangeLog: > > * infcmd.c (default_print_one_register_info): Align natural- > format column value consistently one after another. > --- > gdb/infcmd.c | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index defa7b0..56cdee2 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -2270,6 +2270,15 @@ path_command (char *dirname, int from_tty) > } > > Please add a short comment to explain what this function does. > +static void > +pad_to_column_prepending_tab (string_file& stream, int col) The & should go after the space. > +{ > + const int size_with_tab = stream.size () / 8 * 8 + 8; > + stream.putc ('\t'); > + if (size_with_tab < col) > + stream.puts (n_spaces (col - size_with_tab)); > +} > + > /* Print out the register NAME with value VAL, to FILE, in the default > fashion. */ > > @@ -2280,9 +2289,15 @@ 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, > + value_column_2=32, Perhaps those could have more explicit names, like "column_hex" and "column_natural"? Formatting nit: indent those two lines with two columns less (so with 6 spaces in total) and add spaces around the =. > + }; > > - 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))); > > print_raw_format = (value_entirely_available (val) > && !value_optimized_out (val)); > @@ -2301,14 +2316,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_prepending_tab (format_stream, value_column_2); > + format_stream.puts ("(raw "); > + print_hex_chars (&format_stream, valaddr, TYPE_LENGTH (regtype), > byte_order, > true); > - fprintf_filtered (file, ")"); > + format_stream.puts (")"); > } > } > else > @@ -2320,20 +2336,21 @@ default_print_one_register_info (struct ui_file > *file, > opts.deref_ref = 1; > val_print (regtype, > value_embedded_offset (val), 0, > - file, 0, val, &opts, current_language); > + &format_stream, 0, val, &opts, current_language); > /* If not a vector register, print it also according to its > natural format. */ > if (print_raw_format && TYPE_VECTOR (regtype) == 0) > { > + pad_to_column_prepending_tab (format_stream, value_column_2); > get_user_print_options (&opts); > opts.deref_ref = 1; > - fprintf_filtered (file, "\t"); > val_print (regtype, > value_embedded_offset (val), 0, > - file, 0, val, &opts, current_language); > + &format_stream, 0, val, &opts, current_language); > } > } > > + fputs_filtered (format_stream.c_str (), file); > fprintf_filtered (file, "\n"); > } Thanks, Simon