Re: [PATCH] TUI: Expand TABs into spaces

[Doug Evans <xdje42@gmail.com>]

Eli Zaretskii <eliz@gnu.org> writes:
> Ping! Ping!  OK to install?

"Give me a ping, Vasili. One ping only, please."
ref: http://www.imdb.com/title/tt0099810/quotes
[just a little light humor]

>> Date: Fri, 16 Jan 2015 08:32:15 -0800
>> From: Doug Evans <xdje42@gmail.com>
>> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>> 
>> On Fri, Jan 16, 2015 at 3:17 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> > Ping!  OK to install, master and 7.9 branch?
>> >
>> >> Date: Sat, 03 Jan 2015 13:30:08 +0200
>> >> From: Eli Zaretskii <eliz@gnu.org>
>> >>
>> >> "gdb -tui" relies on the curses library and the underlying terminal
>> >> driver to expand TAB characters into spaces.  But ncurses on Windows
>> >> doesn't do that, and instead displays an IBM graphics character.
>> >>
>> >> The patches below fix that in the command window and in displaying the
>> >> registers.
>> >>
>> >> OK to commit?
>> >>
>> >> 2015-01-03  Eli Zaretskii  <eliz@gnu.org>
>> >>
>> >>       * tui/tui-regs.c (tui_register_format): Expand TABs into the
>> >>       appropriate number of spaces.
>> >>
>> >>       * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs
>> >>       into the appropriate number of spaces.
>> 
>> I'd have to read the patch more to say it's ok,
>> but one thing that is missing are comments
>> explaining *why* we are expanding tabs into spaces.

I check the code (tui-source.c around line 185) and it has this:

                                  else
                                    { /* Store the charcter in the
                                         line buffer.  If it is a tab,
                                         then translate to the correct
                                         number of chars so we don't
                                         overwrite our buffer.  */

So it *does* have a comment explaining why tabs are being expanded.

For tui_puts and tui_redisplay_readline let's add something like
the following:

      /* Windows ncurses doesn't expand tabs, so we have to do that here.  */
      [else] if (c == '\t')

For tui_register_format it used to be such a straightforward function,
it's a shame to grow it by 2x for bitfiddly tab handling.

Let's add a helper routine that takes one string and returns
another with tabs expanded, and put this helper routine in, say tui-io.c
(this routine isn't tui-regs specific), and then call that routine from
tui_register_format.
I didn't review the actual code to do the tab expansion with a microscope.
I'm going to assume it at least mostly works. At least it'll be tucked away.

Also, a note on the ChangeLog:
No blank lines between related entries.

>       * tui/tui-regs.c (tui_register_format): Expand TABs into the
>       appropriate number of spaces.
>
>       * tui/tui-io.c (tui_puts, tui_redisplay_readline): Expand TABs
>       into the appropriate number of spaces.