Re: [PATCH] Make "list" work again in TUI

[Pedro Alves <palves@redhat.com>]

On 09/12/2018 01:20 PM, Pedro Alves wrote:
> On 09/09/2018 07:26 PM, Tom Tromey wrote:
>>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>>
>> Tom> This patch makes the "list" command work again by adding some caching.
>> Tom> Now the TUI tracks the previously displayed frame, PC, and inferior;
>> Tom> and only updates the display if one of these was changed by the
>> Tom> previous command.
>>
>> I have a question about this one now.
>>
>> In the bug the original poster said that to get back to the current
>> location, he would use the "frame" command.  This doesn't work with my
>> patch, and I'm not sure how it would have worked in the past.
> 
> I used to do that too.
> 
>>
>> One part of how the TUI links to the CLI is very convoluted -- there is
>> a special case in tui-out to notice how the "list" command emits fields
>> and newlines and to use that to refresh the window.  I'd rather not do
>> anything this roundabout and fragile...
> 
> Yeah.
> 
>>
>> Other parts of the CLI use #ifdef TUI to decide what action to take when
>> the TUI is available.  This seems like an "old school" approach though I
>> must say I prefer its directness.  I wouldn't mind rewriting "list" to
>> do this.
>>
>> Another option might be to have the frame command unconditionally notify
>> some observer.  Then the TUI could listen for this.
>>
>> Anyway, I'd appreciate comments on which approach CLI/TUI integration
>> ought to ideally take.  I don't know the history here and there aren't
>> guiding comments that I could find.
> 
> I'm under the impression that the TUI was added along with the
> major HP dump/merge back in the day, meaning original design history
> is probably lost.  I may be wrong.
> 
> Offhand, it seems to me that the TUI should update its listed source
> whenever the CLI's current source changes.  I.e., the TUI's source
> window should be updated whenever set_current_source_symtab_and_line is
> used to update the current source & line for the CLI's "list".  Which
> suggests that for "frame" TUI re-centering, an observer notification from
> within set_current_source_symtab_and_line would work.
> 
> If we always updated the source window from a TUI observer for
> that notification, could we make tui_refresh_frame_and_register_information
> just never update the source window?  I.e., remove the select_source_symtab
> call from tui_refresh_frame_and_register_information or something along
> those lines.  That assumes that if the frame changed, then presumably,
> set_current_source_symtab_and_line will have already been called, which
> it seems is true.  (if it isn't, then "list" won't be updated either, and
> we're back to the argument about keeping "list" and the TUI in sync.).
> That would eliminate the need for the caching.
> 

Something to keep in mind, though, is avoiding refreshes/updates 
caused by temporary thread stops while the target is running.
E.g., if we have breakpoints with conditions that eval false,
or e.g. a software watchpoint, we don't want the intermediate
stops to cause a flurry of source window refreshes.  I'm not
sure whether that would invalidate what I said above, depends on
when set_current_source_symtab_and_line is called, I suppose,
but now I'm thinking that it probably does.

That might suggest instead to start by seeing about trying to remove
the select_source_symtab call from tui_refresh_frame_and_register_information.
Why do we need that?  Can we remove it and instead see about making
tui_refresh_frame_and_register_information update the source window
from get_current_source_symtab_and_line, if set_current_source_symtab_and_line
was meanwhile called?  I.e., refresh the source window if something changed
"list"'s current source&line.

Thanks,
Pedro Alves