diff mbox

[RFC] Call tui_before_prompt in do_scroll_vertical

Message ID 20191223012329.GG3865@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Dec. 23, 2019, 1:23 a.m. UTC 
  * Tom Tromey <tom@tromey.com> [2019-12-22 17:03:45 -0700]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> >> 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 wonder if we really need to change the current source symtab.
> If so, I guess the TUI could just call
> set_current_source_symtab_and_line directly.
> 
> The patch below removes the call to print_source_lines -- this approach
> just seems too roundabout for my taste.

I started playing with similar solutions.  I also started looking at
the role tui_refresh_frame_and_register_information plays in keeping
the source/asm windows up to date.  My conclusion was it does nothing
but add more complication.

Tom, the patch below applies on top of yours, it removes all of the
source/asm updating from tui_refresh_frame_and_register_information.
There's some direct action taken in the symtab changed observer
hook.

I also now call update_source_window_as_is on all source windows after
the vertical scroll - this partly fixes the asm/source scroll
synchronisation issue Hannes pointed out.  Calling the *_as_is version
has (I think) an added benefit - if you scroll horizontally, and then
scroll up/down, the horizontal scroll is maintained.  This feels much
nicer to me, and is what caused me to remove the code from
tui_refresh_frame_and_register_information.

I said partly fixes the scroll synchronisation issue as scrolling the
asm window doesn't (and never has I think) caused the source window to
scroll - it kind of feels like it should though... this probably just
requires us to add something similar to the asm windows vert scroll
code.

There is one issue with this patch, but I've run out of time right now
to look at this any further, if you fire up gdb on gdb then do:

  start
  tui enable
  # will be in main now.
  break gdb_main
  continue

The breakpoint line will be the first line in the console, rather than
being in the centre of the window.

I don't know if this is a worth while direction to go in or not, what
do you think?

Thanks,
Andrew


---
diff mbox

Patch

diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 8576bb8fccd..a6fbd88e51b 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -110,18 +110,13 @@  tui_event_modify_breakpoint (struct breakpoint *b)
 
 static bool from_stack;
 
-/* This is set to true if the next window refresh should come from the
-   current source symtab.  */
-
-static bool from_source_symtab;
-
 /* Refresh TUI's frame and register information.  This is a hook intended to be
    used to update the screen after potential frame and register changes.  */
 
 static void
 tui_refresh_frame_and_register_information ()
 {
-  if (!from_stack && !from_source_symtab)
+  if (!from_stack)
     return;
 
   target_terminal::scoped_restore_terminal_state term_state;
@@ -144,6 +139,7 @@  tui_refresh_frame_and_register_information ()
 	  tui_refreshing_registers = 0;
 	}
     }
+#if 0
   else if (!from_stack)
     {
       /* Make sure that the source window is displayed.  */
@@ -152,6 +148,7 @@  tui_refresh_frame_and_register_information ()
       struct symtab_and_line sal = get_current_source_symtab_and_line ();
       tui_update_source_windows_with_line (sal);
     }
+#endif
 }
 
 /* Dummy callback for deprecated_print_frame_info_listing_hook which is called
@@ -184,7 +181,6 @@  tui_before_prompt (const char *current_gdb_prompt)
 {
   tui_refresh_frame_and_register_information ();
   from_stack = false;
-  from_source_symtab = false;
 }
 
 /* Observer for the normal_stop notification.  */
@@ -208,7 +204,11 @@  tui_context_changed (user_selected_what ignore)
 static void
 tui_symtab_changed ()
 {
-  from_source_symtab = true;
+  /* Make sure that the source window is displayed.  */
+  tui_add_win_to_layout (SRC_WIN);
+
+  struct symtab_and_line sal = get_current_source_symtab_and_line ();
+  tui_update_source_windows_with_line (sal);
 }
 
 /* Token associated with observers registered while TUI hooks are
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index bcba9e176f1..4846ddec0fc 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -145,6 +145,7 @@  tui_source_window::do_scroll_vertical (int num_to_scroll)
 	  struct frame_info *fi = get_selected_frame (NULL);
 	  s = find_pc_line_symtab (get_frame_pc (fi));
 	  arch = get_frame_arch (fi);
+	  cursal.symtab = s;
 	}
       else
 	s = cursal.symtab;
@@ -158,7 +159,10 @@  tui_source_window::do_scroll_vertical (int num_to_scroll)
 	line_no = 1;
 
       cursal.line = line_no;
-      update_source_window (arch, cursal);
+      find_line_pc (cursal.symtab, cursal.line, &cursal.pc);
+
+      for (struct tui_source_window_base *win_info : tui_source_windows ())
+	win_info->update_source_window_as_is (arch, cursal);
     }
 }