From: Simon Marchi <simark@simark.ca>
To: Ruslan Kabatsayev <b7.10110111@gmail.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH v3] Align natural-format register values to the same column
Date: Sat, 03 Feb 2018 04:46:00 -0000 [thread overview]
Message-ID: <adf8ddb3-2e2c-4aa0-35cd-74d934723264@simark.ca> (raw)
In-Reply-To: <1516346545-8217-1-git-send-email-b7.10110111@gmail.com>
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 <main+16>
> 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)
> }
> \f
>
> +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
next prev parent reply other threads:[~2018-02-03 4:46 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-19 7:22 Ruslan Kabatsayev
2018-02-03 4:46 ` Simon Marchi [this message]
2018-02-03 11:43 ` Ruslan Kabatsayev
2018-02-03 15:55 ` Simon Marchi
2018-02-03 16:15 ` Ruslan Kabatsayev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=adf8ddb3-2e2c-4aa0-35cd-74d934723264@simark.ca \
--to=simark@simark.ca \
--cc=b7.10110111@gmail.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).