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

[Patrick Palka <patrick at parcs dot ath dot cx>]

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
>