Re: [RFC][PATCH] Call tui_before_prompt in do_scroll_vertical

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Hannes Domani via gdb-patches <gdb-patches@sourceware.org> [2019-12-21 16:33:25 +0100]:

> Without this call scrolling in the src window does not work at all.

Thanks for spotting this and starting work on a fix.

When fixing regressions like this its always useful to track down the
commit that introduced the failure and add a link to it.  This is not
about blaming others, but just adds a really useful historical log.

In this case, it was this commit that caused the breakage in scrolling:

  commit b4b49dcbff6b437fa8b4e2fc0c3f27b457f11310 (refs/bisect/bad)
  Date:   Wed Nov 13 16:47:58 2019 -0700

      Don't call tui_show_source from tui_ui_out

> 
> First I tried it with tui_update_source_windows_with_line, but this didn't
> reset from_source_symtab (which was set deep in print_source_lines),
> which resulted in some weird behavior when switching from "layout split"
> to "layout asm" after scrolling down in the src window (the asm window
> was then overwritten by the src window).

I was able to reproduce the layout split -> layout asm issue you
describe, but, I'm not convinced that the solution below is the right
way to go.

Tom might suggest a better solution quicker, but if not I'll try to
review this code properly soon.

However, the questions/observations I have are:

 - It doesn't feel like calling a before prompt hook is the right way
   to go, and even if this is basically the right solution, we should
   split the important part of the hook into a separate function with
   a suitable name and call that instead.

 - I also don't understand why calling this hook in
   tui_souce_window::do_scroll_vertical changes what happens when we
   switch to tui_disasm_window.  Stepping through from the 'layout
   asm' seems to show that we do correctly change to the asm window,
   and its only when we later call display_gdb_prompt (from
   event-top.c) that we overwrite the asm window with the source for
   some reason.  I'd like to understand what's going on there more as
   it feels like if we could fix that as almost a separate issue then
   we should go back to calling tui_update_source_windows_with_line as
   you tried first, which feels like a better solution.

 - On coding style within GDB, we declare functions in header files,
   and define them in source files.  Also declarations are marked
   extern and include return type and function name on one line, so it
   would be:

   extern void tui_before_prompt (const char *current_gdb_prompt);

   But like I said, I'll need convincing that this is the right
   approach to fix this.

Thanks,
Andrew

> 
> gdb/ChangeLog:
> 
> 2019-12-21  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* tui/tui-hooks.c (tui_before_prompt): Remove static.
> 	* tui/tui-source.c (tui_before_prompt): Add prototype.
> 	(tui_source_window::do_scroll_vertical): Add tui_before_prompt call.
> ---
>  gdb/tui/tui-hooks.c  | 2 +-
>  gdb/tui/tui-source.c | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
> index 8576bb8fcc..8b9e70316f 100644
> --- a/gdb/tui/tui-hooks.c
> +++ b/gdb/tui/tui-hooks.c
> @@ -179,7 +179,7 @@ tui_inferior_exit (struct inferior *inf)
>  
>  /* Observer for the before_prompt notification.  */
>  
> -static void
> +void
>  tui_before_prompt (const char *current_gdb_prompt)
>  {
>    tui_refresh_frame_and_register_information ();
> diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
> index 0728263b8c..dfe5721edf 100644
> --- a/gdb/tui/tui-source.c
> +++ b/gdb/tui/tui-source.c
> @@ -39,6 +39,9 @@
>  #include "tui/tui-source.h"
>  #include "gdb_curses.h"
>  
> +void
> +tui_before_prompt (const char *current_gdb_prompt);
> +
>  /* Function to display source in the source window.  */
>  bool
>  tui_source_window::set_contents (struct gdbarch *arch,
> @@ -158,6 +161,8 @@ tui_source_window::do_scroll_vertical (int num_to_scroll)
>  	l.u.line_no = 1;
>  
>        print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0);
> +
> +      tui_before_prompt ("");
>      }
>  }
>  
> -- 
> 2.24.1
>