From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20947 invoked by alias); 3 Feb 2018 11:43:20 -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 20938 invoked by uid 89); 3 Feb 2018 11:43:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-lf0-f46.google.com Received: from mail-lf0-f46.google.com (HELO mail-lf0-f46.google.com) (209.85.215.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 03 Feb 2018 11:43:18 +0000 Received: by mail-lf0-f46.google.com with SMTP id x196so35260656lfd.12 for ; Sat, 03 Feb 2018 03:43:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=/6DbeK7OG0+OTA8B/CgDIXENa+C0T70y6y/NNTEVUYo=; b=ha9QppInpj3nVR9CTQQhV9qJ2guM5aHeMQ8UDVAhTJA6jKMg7VyKMqVGpc+IzXeRtD woTwReWC/GOdG4z0ud8idWkmj3Li2nZRmcoNDm+a/3mHwbjs3d1HXUkD0TRw9ppp+epX XFomDm92lXjN62h8SpdrRwE5YBCS3bbyCzdHjkClboTsNrfXP1xJKijmYu0s+xxeYMqu CyLCpFeJtSRVkRTDrXMi3RCAQG8Gjuuf49rbR5mjPfQylV+31s2u78Yo+YWjiaUjvhSM SkpyrqHd9LJf+EoNBbVPQO3BUkEHN83bY7YayvZJOEULX6GJ9oAt+bVHHqBSrR1Pcs41 aPGg== X-Gm-Message-State: AKwxytcw8bm72xhy+vuSiApjNPUR4ieds0sGd2gqGgoYxCgMlty5cIrV E20RA6mV1sHMVT+wcxRoBVBabXlhlYc7yI4gJnMviQ== X-Google-Smtp-Source: AH8x227BGC56E6tF17QnPyT0ASaU1RY6UbN4SUtMX4xRZqEQ7ZlKvPrt0iTm5YJuMslLm9STdQv+97S/7QpF2Qyc2IA= X-Received: by 10.25.228.146 with SMTP id x18mr26520896lfi.115.1517658196515; Sat, 03 Feb 2018 03:43:16 -0800 (PST) MIME-Version: 1.0 Received: by 10.25.216.93 with HTTP; Sat, 3 Feb 2018 03:43:16 -0800 (PST) In-Reply-To: References: <1516346545-8217-1-git-send-email-b7.10110111@gmail.com> From: Ruslan Kabatsayev Date: Sat, 03 Feb 2018 11:43:00 -0000 Message-ID: Subject: Re: [PATCH v3] Align natural-format register values to the same column To: Simon Marchi Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00049.txt.bz2 Hi Simon, On 3 February 2018 at 07:46, Simon Marchi wrote: > Hi Ruslan, > > The resulting output looks great. I have a few comments, some of them > are identical to the review of v2. I'm extremely sorry for making you repeat your comments. I keep missing some formatting bits. Are there any known-working options for any linter/indenter so that I could automatically check/prettify my GDB patches before posting? I've tried astyle, but it somehow messes up spaces/tabs and doesn't seem to support half-indenting enum braces (or I didn't use the correct options). GNU indent appears to confuse C++ references with bitwise OR, putting spaces on both sides, and also doesn't want to half-indent enums. I've sent a new version of the patch. > > 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 > Regards, Ruslan