[PATCH 1/2] tui: make updating of start_line in tui_puts more consistent

[PATCH 1/2] tui: make updating of start_line in tui_puts more consistent

[Patrick Palka]

The command window's start_line field is used by the function
tui_redisplay_readline to figure out on which window line to start
redrawing the (possibly multi-line) command line.  It differs from the Y
coordinate of the window cursor only when the length of the line being
outputted is longer than the width of the window.

The function tui_puts however currently does not respect this property
of start_line.  After a call to tui_puts, start_line will always be
equal to cur_line even when the outputted line may have wrapped a few
times.  This patch makes tui_puts update start_line in a way that's
consistent with how tui_redisplay_readline does it.  This patch also
explicitly documents this property of start_line.

gdb/ChangeLog:

	* tui/tui-data.h (tui_command_info): Add comment documenting the
	field start_line.
	* tui/tui-io.c (tui_puts): Fix the updating of start_line.
---
 gdb/tui/tui-data.h |  5 +++++
 gdb/tui/tui-io.c   | 17 ++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index df1fe6c..2f9030a 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -265,6 +265,11 @@ struct tui_source_info
 
 struct tui_command_info
 {
+  /* The line number (i.e. the Y coordinate of the command window) that
+     contains the start of the latest line being outputted.  START_LINE differs
+     from the Y coordinate of the window cursor only when the current line is
+     longer than the width of the window, i.e. only when the current line has
+     wrapped to the next line one or more times.  */
   int start_line;
 };
 
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index c7a092f..311c96c 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -168,7 +168,12 @@ tui_puts (const char *string)
         }
       else if (tui_skip_line != 1)
         {
+	  int prev_line, prev_col;
+
           tui_skip_line = -1;
+
+	  getyx (w, prev_line, prev_col);
+
 	  /* Expand TABs, since ncurses on MS-Windows doesn't.  */
 	  if (c == '\t')
 	    {
@@ -183,11 +188,21 @@ tui_puts (const char *string)
 	    }
 	  else
 	    waddch (w, c);
+
+	  if (c == '\n')
+	    TUI_CMD_WIN->detail.command_info.start_line = getcury (w);
+	  else if (getcurx (w) <= prev_col && getcury (w) == prev_line)
+	    {
+	      /* If the cursor is on the last line of the command window and the
+		 added character caused the line to wrap, then we have to adjust
+		 start_line to compensate for the scrolling up of each line that
+		 the line wrapping caused.  */
+	      TUI_CMD_WIN->detail.command_info.start_line--;
+	    }
         }
       else if (c == '\n')
         tui_skip_line = -1;
     }
-  TUI_CMD_WIN->detail.command_info.start_line = getcury (w);
 }
 
 /* Readline callback.
-- 
2.5.0.417.g69c5dbd.dirty

[PATCH 2/2] tui: maintain a scrollback buffer and dump it upon exit (PR tui/14584)

[Patrick Palka]

It is currently impossible to scroll through the text emitted in the
TUI's command window.  If some output leaves the top of the window then it
is effectively gone forever.

This patch attempts to laterally work around this deficiency not by
implementing command-window scrolling within the TUI, but rather by
accumulating a scrollback buffer of command-window output while the TUI
is active and dumping it into the CLI when the TUI gets disabled.  So
when the user wants to scroll through the output emitted under the TUI,
with this patch the user can just temporarily switch to the CLI
(triggering the verbatim dumping of the window's contents) and then use
their terminal emulator/multiplexer to scroll/search through the output
as if it came from the CLI itself.  This gives the impression that the
TUI command-window and the CLI output are unified (although the
synchronization only goes one way).

The implementation is pretty straightforward.  Whenever a newline is
emitted in the command window, the current line gets copied from the
screen and into a scrollback buffer.  (Care must be taken to account for
long, wrapped lines.  Now that start_line is always valid, this is easy
enough.)  When the TUI is disabled, the buffer gets dumped and then
cleared.

[ Is this a good approach to addressing the lack of scrolling capabilities
  in the TUI command window?  I don't see much advantage to having a
  direct implementation of command-window scrolling compared to this
  approach of piggybacking on the underlying terminal
  emulator/multiplexer.  Though such an approach and the approach this
  patch takes are not mutually exclusive anyway.  ]

gdb/ChangeLog:

	* tui/tui-io.h (tui_dump_scrollback_buffer): Declare.
	* tui/tui-io.c (tui_dump_scrollback_buffer): Define.
	(tui_scrollback_buffer): Define.
	(tui_add_current_line_to_scrollback_buffer): Define.
	(tui_puts): Call it before emitting a newline.
	* tui/tui.c (tui_disable): Call tui_dump_scrollback_buffer.
---
 gdb/tui/tui-io.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/tui/tui-io.h |  5 +++++
 gdb/tui/tui.c    |  3 +++
 3 files changed, 66 insertions(+)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 311c96c..b3376ac 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -132,6 +132,11 @@ static FILE *tui_old_rl_outstream;
 static int tui_readline_pipe[2];
 #endif
 
+/* The scrollback buffer containing a line-by-line copy of all that's been
+   outputted to the command window during the current TUI session.  */
+
+static VEC (char_ptr) *tui_scrollback_buffer = NULL;
+
 /* The last gdb prompt that was registered in readline.
    This may be the main gdb prompt or a secondary prompt.  */
 static char *tui_rl_saved_prompt;
@@ -146,6 +151,56 @@ tui_putc (char c)
   tui_puts (buf);
 }
 
+/* See tui-io.h.  */
+
+void
+tui_dump_scrollback_buffer (void)
+{
+  int i;
+  for (i = 0; i < VEC_length (char_ptr, tui_scrollback_buffer); i++)
+    {
+      char *line = VEC_index (char_ptr, tui_scrollback_buffer, i);
+      fputs_unfiltered (line, gdb_stdout);
+      fputc_unfiltered ('\n', gdb_stdout);
+      xfree (line);
+    }
+
+  VEC_free (char_ptr, tui_scrollback_buffer);
+}
+
+/* Copy the current line, delimited by the screen coordinates
+   (START_LINE, 0) to the current cursor position (CUR_LINE, CUR_X), to the
+   scrollback buffer.  */
+
+static void
+tui_add_current_line_to_scrollback_buffer (void)
+{
+  WINDOW *w = TUI_CMD_WIN->generic.handle;
+  const int start_line = TUI_CMD_WIN->detail.command_info.start_line;
+  const int cur_line = getcury (w);
+  const int cur_x = getcurx (w);
+  const int max_x = getmaxx (w);
+  int i, j, k;
+  char *line;
+
+  wmove (w, cur_line, cur_x);
+
+  /* Allocate enough space to hold the entire (possibly wrapped) line.  */
+  line = xcalloc ((cur_line - start_line) * max_x + cur_x + 2, sizeof (*line));
+
+  k = 0;
+  for (j = start_line; j < cur_line; j++)
+    for (i = 0; i < max_x; i++)
+      line[k++] = mvwinch (w, j, i);
+
+  for (i = 0; i < cur_x; i++)
+    line[k++] = mvwinch (w, cur_line, i);
+  line[k++] = '\0';
+
+  VEC_safe_push (char_ptr, tui_scrollback_buffer, line);
+  wmove (w, cur_line, cur_x);
+}
+
 /* Print the string in the curses command window.
    The output is buffered.  It is up to the caller to refresh the screen
    if necessary.  */
@@ -172,6 +227,9 @@ tui_puts (const char *string)
 
           tui_skip_line = -1;
 
+	  if (c == '\n')
+	    tui_add_current_line_to_scrollback_buffer ();
+
 	  getyx (w, prev_line, prev_col);
 
 	  /* Expand TABs, since ncurses on MS-Windows doesn't.  */
diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h
index 3154eee..e61ed14 100644
--- a/gdb/tui/tui-io.h
+++ b/gdb/tui/tui-io.h
@@ -44,6 +44,11 @@ extern void tui_redisplay_readline (void);
 /* Expand TABs into spaces.  */
 extern char *tui_expand_tabs (const char *, int);
 
+/* Emit the scrollback buffer accumulated by the command window in its
+   entirety, and then clear it.  */
+
+extern void tui_dump_scrollback_buffer (void);
+
 extern struct ui_out *tui_out;
 extern struct ui_out *tui_old_uiout;
 
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 838471d..69ed60b 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -538,6 +538,9 @@ tui_disable (void)
 
   tui_active = 0;
   tui_update_gdb_sizes ();
+
+  /* Dump the command window's scrollback buffer.  */
+  tui_dump_scrollback_buffer ();
 }
 
 /* Command wrapper for enabling tui mode.  */
-- 
2.5.0.417.g69c5dbd.dirty

Re: [PATCH 2/2] tui: maintain a scrollback buffer and dump it upon exit (PR tui/14584)

[Pedro Alves]

On 08/26/2015 02:06 AM, Patrick Palka wrote:
> It is currently impossible to scroll through the text emitted in the
> TUI's command window.  If some output leaves the top of the window then it
> is effectively gone forever.
> 
> This patch attempts to laterally work around this deficiency not by
> implementing command-window scrolling within the TUI, but rather by
> accumulating a scrollback buffer of command-window output while the TUI
> is active and dumping it into the CLI when the TUI gets disabled.  So
> when the user wants to scroll through the output emitted under the TUI,
> with this patch the user can just temporarily switch to the CLI
> (triggering the verbatim dumping of the window's contents) and then use
> their terminal emulator/multiplexer to scroll/search through the output
> as if it came from the CLI itself.  This gives the impression that the
> TUI command-window and the CLI output are unified (although the
> synchronization only goes one way).
> 
> The implementation is pretty straightforward.  Whenever a newline is
> emitted in the command window, the current line gets copied from the
> screen and into a scrollback buffer.  (Care must be taken to account for
> long, wrapped lines.  Now that start_line is always valid, this is easy
> enough.)  When the TUI is disabled, the buffer gets dumped and then
> cleared.
> 
> [ Is this a good approach to addressing the lack of scrolling capabilities
>   in the TUI command window?  I don't see much advantage to having a
>   direct implementation of command-window scrolling compared to this
>   approach of piggybacking on the underlying terminal
>   emulator/multiplexer.  Though such an approach and the approach this
>   patch takes are not mutually exclusive anyway.  ]

I have to admit that it took me a bit longer to look at this one,
as it feels like a hack to me.  It'd feel more natural to me to
scroll without leaving the TUI.  But dumping is definitely better
than what we have, so I can't really object it.

> 
> gdb/ChangeLog:
> 
> 	* tui/tui-io.h (tui_dump_scrollback_buffer): Declare.
> 	* tui/tui-io.c (tui_dump_scrollback_buffer): Define.
> 	(tui_scrollback_buffer): Define.
> 	(tui_add_current_line_to_scrollback_buffer): Define.
> 	(tui_puts): Call it before emitting a newline.
> 	* tui/tui.c (tui_disable): Call tui_dump_scrollback_buffer.
> ---
>  gdb/tui/tui-io.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/tui/tui-io.h |  5 +++++
>  gdb/tui/tui.c    |  3 +++
>  3 files changed, 66 insertions(+)
> 
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index 311c96c..b3376ac 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -132,6 +132,11 @@ static FILE *tui_old_rl_outstream;
>  static int tui_readline_pipe[2];
>  #endif
>  
> +/* The scrollback buffer containing a line-by-line copy of all that's been
> +   outputted to the command window during the current TUI session.  */
> +
> +static VEC (char_ptr) *tui_scrollback_buffer = NULL;
> +
>  /* The last gdb prompt that was registered in readline.
>     This may be the main gdb prompt or a secondary prompt.  */
>  static char *tui_rl_saved_prompt;
> @@ -146,6 +151,56 @@ tui_putc (char c)
>    tui_puts (buf);
>  }
>  
> +/* See tui-io.h.  */
> +
> +void
> +tui_dump_scrollback_buffer (void)
> +{
> +  int i;

Add empty line here.

> +  for (i = 0; i < VEC_length (char_ptr, tui_scrollback_buffer); i++)
> +    {
> +      char *line = VEC_index (char_ptr, tui_scrollback_buffer, i);
> +      fputs_unfiltered (line, gdb_stdout);
> +      fputc_unfiltered ('\n', gdb_stdout);
> +      xfree (line);
> +    }
> +
> +  VEC_free (char_ptr, tui_scrollback_buffer);
> +}
> +
> +/* Copy the current line, delimited by the screen coordinates
> +   (START_LINE, 0) to the current cursor position (CUR_LINE, CUR_X), to the
> +   scrollback buffer.  */
> +
> +static void
> +tui_add_current_line_to_scrollback_buffer (void)
> +{
> +  WINDOW *w = TUI_CMD_WIN->generic.handle;
> +  const int start_line = TUI_CMD_WIN->detail.command_info.start_line;
> +  const int cur_line = getcury (w);
> +  const int cur_x = getcurx (w);
> +  const int max_x = getmaxx (w);
> +  int i, j, k;
> +  char *line;
> +
> +  wmove (w, cur_line, cur_x);
> +
> +  /* Allocate enough space to hold the entire (possibly wrapped) line.  */
> +  line = xcalloc ((cur_line - start_line) * max_x + cur_x + 2, sizeof (*line));
> +
> +  k = 0;
> +  for (j = start_line; j < cur_line; j++)
> +    for (i = 0; i < max_x; i++)
> +      line[k++] = mvwinch (w, j, i);
> +
> +  for (i = 0; i < cur_x; i++)
> +    line[k++] = mvwinch (w, cur_line, i);
> +  line[k++] = '\0';
> +
> +  VEC_safe_push (char_ptr, tui_scrollback_buffer, line);
> +  wmove (w, cur_line, cur_x);
> +}

I'm surprised by this though.  This seems backwards -- extracting
data out of the presentation layer.  Can't it be tui_puts that pushes
data in the scrollback buffer?

Thanks,
Pedro Alves

Re: [PATCH 2/2] tui: maintain a scrollback buffer and dump it upon exit (PR tui/14584)

[Patrick Palka]

On Thu, Sep 10, 2015 at 11:25 AM, Pedro Alves <palves@redhat.com> wrote:
> On 08/26/2015 02:06 AM, Patrick Palka wrote:
>> It is currently impossible to scroll through the text emitted in the
>> TUI's command window.  If some output leaves the top of the window then it
>> is effectively gone forever.
>>
>> This patch attempts to laterally work around this deficiency not by
>> implementing command-window scrolling within the TUI, but rather by
>> accumulating a scrollback buffer of command-window output while the TUI
>> is active and dumping it into the CLI when the TUI gets disabled.  So
>> when the user wants to scroll through the output emitted under the TUI,
>> with this patch the user can just temporarily switch to the CLI
>> (triggering the verbatim dumping of the window's contents) and then use
>> their terminal emulator/multiplexer to scroll/search through the output
>> as if it came from the CLI itself.  This gives the impression that the
>> TUI command-window and the CLI output are unified (although the
>> synchronization only goes one way).
>>
>> The implementation is pretty straightforward.  Whenever a newline is
>> emitted in the command window, the current line gets copied from the
>> screen and into a scrollback buffer.  (Care must be taken to account for
>> long, wrapped lines.  Now that start_line is always valid, this is easy
>> enough.)  When the TUI is disabled, the buffer gets dumped and then
>> cleared.
>>
>> [ Is this a good approach to addressing the lack of scrolling capabilities
>>   in the TUI command window?  I don't see much advantage to having a
>>   direct implementation of command-window scrolling compared to this
>>   approach of piggybacking on the underlying terminal
>>   emulator/multiplexer.  Though such an approach and the approach this
>>   patch takes are not mutually exclusive anyway.  ]
>
> I have to admit that it took me a bit longer to look at this one,
> as it feels like a hack to me.  It'd feel more natural to me to
> scroll without leaving the TUI.  But dumping is definitely better
> than what we have, so I can't really object it.

I agree that it'd feel more natural to scroll without leaving the TUI.
Let's forget about this part of the patch for now :) I am not very
keen about it anymore, and I realized that there is something more
reasonable I can implement alongside the scrollback buffer
implementation (and that is the automatic redrawing of the command
window's contents following a screen resize).

>
>>
>> gdb/ChangeLog:
>>
>>       * tui/tui-io.h (tui_dump_scrollback_buffer): Declare.
>>       * tui/tui-io.c (tui_dump_scrollback_buffer): Define.
>>       (tui_scrollback_buffer): Define.
>>       (tui_add_current_line_to_scrollback_buffer): Define.
>>       (tui_puts): Call it before emitting a newline.
>>       * tui/tui.c (tui_disable): Call tui_dump_scrollback_buffer.
>> ---
>>  gdb/tui/tui-io.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  gdb/tui/tui-io.h |  5 +++++
>>  gdb/tui/tui.c    |  3 +++
>>  3 files changed, 66 insertions(+)
>>
>> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
>> index 311c96c..b3376ac 100644
>> --- a/gdb/tui/tui-io.c
>> +++ b/gdb/tui/tui-io.c
>> @@ -132,6 +132,11 @@ static FILE *tui_old_rl_outstream;
>>  static int tui_readline_pipe[2];
>>  #endif
>>
>> +/* The scrollback buffer containing a line-by-line copy of all that's been
>> +   outputted to the command window during the current TUI session.  */
>> +
>> +static VEC (char_ptr) *tui_scrollback_buffer = NULL;
>> +
>>  /* The last gdb prompt that was registered in readline.
>>     This may be the main gdb prompt or a secondary prompt.  */
>>  static char *tui_rl_saved_prompt;
>> @@ -146,6 +151,56 @@ tui_putc (char c)
>>    tui_puts (buf);
>>  }
>>
>> +/* See tui-io.h.  */
>> +
>> +void
>> +tui_dump_scrollback_buffer (void)
>> +{
>> +  int i;
>
> Add empty line here.
>
>> +  for (i = 0; i < VEC_length (char_ptr, tui_scrollback_buffer); i++)
>> +    {
>> +      char *line = VEC_index (char_ptr, tui_scrollback_buffer, i);
>> +      fputs_unfiltered (line, gdb_stdout);
>> +      fputc_unfiltered ('\n', gdb_stdout);
>> +      xfree (line);
>> +    }
>> +
>> +  VEC_free (char_ptr, tui_scrollback_buffer);
>> +}
>> +
>> +/* Copy the current line, delimited by the screen coordinates
>> +   (START_LINE, 0) to the current cursor position (CUR_LINE, CUR_X), to the
>> +   scrollback buffer.  */
>> +
>> +static void
>> +tui_add_current_line_to_scrollback_buffer (void)
>> +{
>> +  WINDOW *w = TUI_CMD_WIN->generic.handle;
>> +  const int start_line = TUI_CMD_WIN->detail.command_info.start_line;
>> +  const int cur_line = getcury (w);
>> +  const int cur_x = getcurx (w);
>> +  const int max_x = getmaxx (w);
>> +  int i, j, k;
>> +  char *line;
>> +
>> +  wmove (w, cur_line, cur_x);
>> +
>> +  /* Allocate enough space to hold the entire (possibly wrapped) line.  */
>> +  line = xcalloc ((cur_line - start_line) * max_x + cur_x + 2, sizeof (*line));
>> +
>> +  k = 0;
>> +  for (j = start_line; j < cur_line; j++)
>> +    for (i = 0; i < max_x; i++)
>> +      line[k++] = mvwinch (w, j, i);
>> +
>> +  for (i = 0; i < cur_x; i++)
>> +    line[k++] = mvwinch (w, cur_line, i);
>> +  line[k++] = '\0';
>> +
>> +  VEC_safe_push (char_ptr, tui_scrollback_buffer, line);
>> +  wmove (w, cur_line, cur_x);
>> +}
>
> I'm surprised by this though.  This seems backwards -- extracting
> data out of the presentation layer.  Can't it be tui_puts that pushes
> data in the scrollback buffer?

Yeah, it is backwards.  I wrote it this way so that I wouldn't have to
worry about how to intermingle the prompt line with the subsequent
output line when adding a line to the scrollback buffer since it is
not always the case that the prompt line and the subsequent output
line are separated by a newline, e.g. when the user presses ^C while
still at the prompt.  At the time this seemed tricky but in the middle
of writing this I think I have figured out a less backwards approach
that would still handle such cases.  I will post a revised patch soon.




>
> Thanks,
> Pedro Alves
>

Re: [PATCH 2/2] tui: maintain a scrollback buffer and dump it upon exit (PR tui/14584)

[Pedro Alves]

On 09/10/2015 11:56 PM, Patrick Palka wrote:

> I will post a revised patch soon.

Alright, looking forward.  Thanks!

-- 
Pedro Alves

Re: [PATCH 1/2] tui: make updating of start_line in tui_puts more consistent

[Pedro Alves]

On 08/26/2015 02:06 AM, Patrick Palka wrote:
> The command window's start_line field is used by the function
> tui_redisplay_readline to figure out on which window line to start
> redrawing the (possibly multi-line) command line.  It differs from the Y
> coordinate of the window cursor only when the length of the line being
> outputted is longer than the width of the window.
> 
> The function tui_puts however currently does not respect this property
> of start_line.  After a call to tui_puts, start_line will always be
> equal to cur_line even when the outputted line may have wrapped a few
> times.  This patch makes tui_puts update start_line in a way that's
> consistent with how tui_redisplay_readline does it.  This patch also
> explicitly documents this property of start_line.
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-data.h (tui_command_info): Add comment documenting the
> 	field start_line.
> 	* tui/tui-io.c (tui_puts): Fix the updating of start_line.

OK.

Thanks,
Pedro Alves