TUI: Expand TABs into spaces

  "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.
  

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.
> 
> 
> --- gdb/tui/tui-io.c~0	2014-10-29 21:45:50.000000000 +0200
> +++ gdb/tui/tui-io.c	2015-01-03 11:12:52.187500000 +0200
> @@ -179,7 +179,19 @@ tui_puts (const char *string)
>        else if (tui_skip_line != 1)
>          {
>            tui_skip_line = -1;
> -          waddch (w, c);
> +	  if (c == '\t')
> +	    {
> +	      int line, col;
> +
> +	      getyx (w, line, col);
> +	      do
> +		{
> +		  waddch (w, ' ');
> +		  col++;
> +		} while ((col % 8) != 0);
> +	    }
> +	  else
> +	    waddch (w, c);
>          }
>        else if (c == '\n')
>          tui_skip_line = -1;
> @@ -254,6 +266,15 @@ tui_redisplay_readline (void)
>            waddch (w, '^');
>            waddch (w, CTRL_CHAR (c) ? UNCTRL (c) : '?');
>  	}
> +      else if (c == '\t')
> +	{
> +	  getyx (w, line, col);
> +	  do
> +	    {
> +	      waddch (w, ' ');
> +	      col++;
> +	    } while ((col % 8) != 0);
> +	}
>        else
>  	{
>            waddch (w, c);
> 
> 
> --- gdb/tui/tui-regs.c~0	2014-10-29 21:45:50.000000000 +0200
> +++ gdb/tui/tui-regs.c	2015-01-03 12:52:42.062500000 +0200
> @@ -676,8 +676,9 @@ tui_register_format (struct frame_info *
>    struct ui_file *stream;
>    struct ui_file *old_stdout;
>    struct cleanup *cleanups;
> -  char *p, *s;
> +  char *p, *s, *q;
>    char *ret;
> +  int n_adjust, col;
>  
>    pagination_enabled = 0;
>    old_stdout = gdb_stdout;
> @@ -694,7 +695,47 @@ tui_register_format (struct frame_info *
>    if (s && s[1] == 0)
>      *s = 0;
>  
> -  ret = xstrdup (p);
> +  /* Expand tabs into spaces.  */
> +  /* 1. How many additional characters do we need?  */
> +  for (col = n_adjust = 0, s = p; s; )
> +    {
> +      s = strpbrk (s, "\t");
> +      if (s)
> +	{
> +	  col = (s - p) + n_adjust;
> +	  /* Adjustment for the next tab stop, minus one for the TAB
> +	     we replace with spaces.  */
> +	  n_adjust += 8 - (col % 8) - 1;
> +	  s++;
> +	}
> +    }
> +
> +  /* Allocate the copy.  */
> +  ret = q = xmalloc (strlen (p) + n_adjust + 1);
> +
> +  /* 2. Copy the original string while replacing TABs with spaces.  */
> +  for (col = 0, s = p; s; )
> +    {
> +      char *s1 = strpbrk (s, "\t");
> +      if (s1)
> +	{
> +	  if (s1 > s)
> +	    {
> +	      strncpy (q, s, s1 - s);
> +	      q += s1 - s;
> +	      col += s1 - s;
> +	    }
> +	  do {
> +	    *q++ = ' ';
> +	    col++;
> +	  } while ((col % 8) != 0);
> +	  s1++;
> +	}
> +      else
> +	strcpy (q, s);
> +      s = s1;
> +    }
> +
>    do_cleanups (cleanups);
>  
>    return ret;
>
  

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.
  

> Date: Fri, 16 Jan 2015 08:32:15 -0800
> From: Doug Evans <xdje42@gmail.com>
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> >> 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.

You mean, the fact that the Windows port of ncurses doesn't?  Sure,
can do that.  But note that a similar feature in displaying the source
doesn't have any such comments.
  

On Fri, Jan 16, 2015 at 8:43 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 16 Jan 2015 08:32:15 -0800
>> From: Doug Evans <xdje42@gmail.com>
>> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>
>> >> 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.
>
> You mean, the fact that the Windows port of ncurses doesn't?  Sure,
> can do that.  But note that a similar feature in displaying the source
> doesn't have any such comments.

Is that in relation to the tabset command?
And is that to provide a knob so that source written
with an expectation that \t was different than 8
can be made readable?

Or is there existing code that is coping with these
windows, umm, quirks?
  

> Date: Fri, 16 Jan 2015 09:29:52 -0800
> From: Doug Evans <xdje42@gmail.com>
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> Is that in relation to the tabset command?
> And is that to provide a knob so that source written
> with an expectation that \t was different than 8
> can be made readable?

No.

> Or is there existing code that is coping with these
> windows, umm, quirks?

Yes.
  

On Fri, Jan 16, 2015 at 9:53 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 16 Jan 2015 09:29:52 -0800
>> From: Doug Evans <xdje42@gmail.com>
>> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>
>> Is that in relation to the tabset command?
>> And is that to provide a knob so that source written
>> with an expectation that \t was different than 8
>> can be made readable?
>
> No.
>
>> Or is there existing code that is coping with these
>> windows, umm, quirks?
>
> Yes.

Good to know.
Can you point me at it?
  

> Date: Fri, 16 Jan 2015 10:25:10 -0800
> From: Doug Evans <xdje42@gmail.com>
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> >> Or is there existing code that is coping with these
> >> windows, umm, quirks?
> >
> > Yes.
> 
> Good to know.
> Can you point me at it?

Point you at what?
  

On Fri, Jan 16, 2015 at 12:11 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 16 Jan 2015 10:25:10 -0800
>> From: Doug Evans <xdje42@gmail.com>
>> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>
>> >> Or is there existing code that is coping with these
>> >> windows, umm, quirks?
>> >
>> > Yes.
>>
>> Good to know.
>> Can you point me at it?
>
> Point you at what?

The "similar feature in displaying the source
doesn't have any such comments."

I thought I found it, but then wasn't sure,
and I don't want to guess.
  

> Date: Fri, 16 Jan 2015 17:02:27 -0800
> From: Doug Evans <xdje42@gmail.com>
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> >> Can you point me at it?
> >
> > Point you at what?
> 
> The "similar feature in displaying the source
> doesn't have any such comments."

Ah, okay.

tui-source.c:tui_set_source_content, around line 185 of tui-source.c,
is what I had in mind.
  

Ping! Ping!  OK to install?

> 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.
>
  

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.