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

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

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.

>
> Thanks,
> Pedro Alves
>