Re: [PATCH 3/3] Replace TUI's select_frame hook (PR tui/13378)

[Patrick Palka <patrick@parcs.ath.cx>]

On Tue, Jun 30, 2015 at 10:54 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Tue, Jun 30, 2015 at 10:08 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 06/30/2015 01:37 PM, Patrick Palka wrote:
>>> This version adds a tui_normal_stop observer in place of augmenting the
>>> tui_on_sync_execution_done observer.  And tui_refresh_frame_and_register_information
>>> can now be made a static function.
>>>
>>> The observer takes a print_frame parameter that is supposed to inform us
>>> whether the frame should be printed.  This boolean seems to only be true for
>>
>> s/only be true/only be false/
>>
>>> when the inferior has exited.  Since tui_refresh_frame_and_register_information
>>> already handles this case by checking has_stack_frames() this patch elects to
>>> ignore this parameter in the observer.
>>
>> This is OK.  I'll take a look at patch 2 soon.
>>
>> Did you find that we still need deprecated_print_frame_info_listing_hook?
>
> It still seems to be "necessary" -- at least, I can't outright remove it.
>
> The only caller of deprecated_print_frame_info_listing_hook is in
> print_frame_info and its use looks like this:
>
>  if (deprecated_print_frame_info_listing_hook)
>     (*deprecated_print_frame_info_listing_hook) (...);
>  else
>    { ... other code ... }
>
> If I remove the hook by replacing the above code with
>
>   { ... other code ... }
>
> Then a regression occurs: the TUI decides to make sure that the
> currently executing line always sits at the top of the window instead
> of only scrolling the screen when the currently executing line is not
> invisible.
>
> But if I disable the hook by replacing the body of code in print_frame_info with
>
>   if (deprecated_print_frame_info_listing_hook)
>     ;
>   else
>     { ... other code ... }
>
> Then everything seems to be OK.  So the code in  the else branch is
> interfering with TUI somehow.  I will investigate further.

The call to "print_source_lines (sal.symtab, sal.line, sal.line + 1,
0);" in the else branch eventually calls "tui_show_source (sal.line);"
which adjusts the source window so that sal.line is the very first
line visible.

I'm not sure how easy this would be to fix properly.  We want to avoid
calling print_source_lines in print_frame_info when the TUI is active.
Of course, I can just guard the code with "if (tui_active)" but that's
not a good fix.  Instead of removing the hook yet, what about making
it (tui_print_frame_info_listing_hook) a no-op in the interim?

BTW, in the CLI, this call to print_source_lines is responsible for
printing the stopped-at source line to stdout, e.g.

(gdb) start
....
Temporary breakpoint 2, main () at 13378.c:9
9         int i = 0;  // THIS LINE
(gdb)