From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88684 invoked by alias); 23 Jul 2017 19:46:35 -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 87882 invoked by uid 89); 23 Jul 2017 19:46:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.7 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= 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; Sun, 23 Jul 2017 19:46:32 +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 v6NJkPPx012913 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sun, 23 Jul 2017 15:46:30 -0400 Received: by simark.ca (Postfix, from userid 112) id 5FC481E5E6; Sun, 23 Jul 2017 15:46:25 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id C4E431E194; Sun, 23 Jul 2017 15:46:23 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 23 Jul 2017 19:46:00 -0000 From: Simon Marchi To: Ruslan Kabatsayev Cc: gdb-patches@sourceware.org Subject: Re: [PING][PATCH] Align natural-format register values to the same column In-Reply-To: References: <1499607122-11131-1-git-send-email-b7.10110111@gmail.com> Message-ID: <7e394ecee6241eb437c8478bf618a2a7@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 Sun, 23 Jul 2017 19:46:25 +0000 X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00340.txt.bz2 Hi Ruslan, Thanks for the patch, it does indeed look pretty bad right now. If we are going to do the alignment by hand using spaces (which seems like a good idea, as it gives more control), I'd rather not keep the tab as an historic artifact. How many tests would have to be fixed, and how difficult would it be? >> 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 | 27 ++++++++++++++++++--------- >> 1 file changed, 18 insertions(+), 9 deletions(-) >> >> diff --git a/gdb/infcmd.c b/gdb/infcmd.c >> index defa7b0..5de4e68 100644 >> --- a/gdb/infcmd.c >> +++ b/gdb/infcmd.c >> @@ -2280,9 +2280,10 @@ default_print_one_register_info (struct ui_file >> *file, >> { >> struct type *regtype = value_type (val); >> int print_raw_format; >> + string_file format_stream; >> >> - fputs_filtered (name, file); >> - print_spaces_filtered (15 - strlen (name), file); >> + format_stream.puts (name); >> + format_stream.puts (n_spaces (15 - strlen (name))); >> >> print_raw_format = (value_entirely_available (val) >> && !value_optimized_out (val)); >> @@ -2301,14 +2302,18 @@ 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, >> + const int size_with_tab = format_stream.size () / 8 * 8 + 8; >> + format_stream.putc ('\t'); >> + if (size_with_tab < 32) >> + format_stream.puts (n_spaces (32 - size_with_tab)); Could you extract that constant (32) to a const variable with some meaningful name? If we ever change the width, I predict that we'll forget to update the instances of the same 32 down there. The padding could also be made into a small function to avoid repeating the code. >> + 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 +2325,24 @@ 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) >> { >> + const int size_with_tab = format_stream.size () / 8 * 8 + 8; >> + format_stream.putc ('\t'); >> + if (size_with_tab < 32) >> + format_stream.puts (n_spaces (32 - size_with_tab)); >> 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