[Lazarus] Events to be more frontend centric [[was: Re: Playing with debuggers]]

[Martin Frb]

> 1) Events to be more frontend centric.
> 2) Actions (eval watch) to be triggered by the frontend
> 3) Representation of an "watch"  (Should methods be on the Watch 
> object, or the Debugger object)

=====> 1) Events to be more frontend centric.

The current events and especially states are certainly in need of clean up.

I would not say that they are all backend centric.
States like: dsRun, dsPause, dsStop  are very valid and meaningful for 
the frontends too.

On the other hand states like
- dsDestroying is backend only, and should not be exposed.
- dsInit, dsInternalPause are not well defined at all.


Looking at your code, I do not agree with your new states.
>   TLazExtEvaluationState = (
>     esNone,                  // No debugger available
>     esEvaluationImpossible,  // Evaluation of variables is impossible 
> (Maybe because there is nu debugee? (stopped))
>     esEvaluationPaused,      // Evaluation of variables is temporarily 
> disabled (Maybe because debugee is running?)
>     esEvaluationPossible     // Evaluation of variables is possible 
> (Debugee is probably paused)
>   );

1 - First of all,
The above are either
a- not enough
b- to many

a) There may be different extends of evaluation in the frontend. So more 
info than the above few states is needed.
=> Of course, the states may be sufficient, as further info could be 
maintained within the frontend.
* I quite often use breakpoints, that do not "break" (continue 
automatically), but take a history snapshot.
Currently when this happens the watch/stack/.. windows actually do 
update (and that is a workaround to missing options).
But ideally they should not. Only the history-snapshot should run.
* Or while viewing history => one can get existing values, but not 
evaluate new ones.

b) On the other hand, why differ between Paused and Impossible? They 
both mean "can't eval".
Your frontend may differ between the 2 reasons behind them. But other 
Frontends, relay on other factors.

After writing all the below, I realized that (probably) your frontend 
uses the 2 states to decide, if there should be a timeout before 
clearing the values?
But that is very specific to your frontend (well most frontends probably 
would benefit). But then this is just a duplication of the existing and 
further needed dsStop.

See idea on "CurrentCapacities" below.


2 - Second,
they already exist:
    WatchValue.Validity => ddsUnknown, ddsRequested, ddsEvaluating,  
ddsValid, ddsInvalid, ddsError
Though they are only set, once a value gets requested.
(Asking upfront, may be an idea, but one only need "function CanEval: 
boolean". / See idea on "CurrentCapacities" below.)

This falls partly under the topic (3) representation => should the state 
come via the watch or the debugger.

And then, if history is shown, there is no point of calling a function 
on the debugger. There is no data to be computed by the debugger.


3 - Third
Also, while your argument for a frontend-needs driven event system is 
compiling, we need to keep in mind, that the frontend (or plural 
frontends) may change/differ how they want to react to different backend 
states.
The backends can not actually know this. So the frontend still needs 
(most of) the DebuggerStates (dsPause, dsRun, dsStopped)

Yet, as mentioned above, (sub)states may need to be added for different 
"pause reasons".
This may be something (some/all) frontends can do on their own. But some 
may also belong into the backend. (Maybe such as, that the backend is 
going to continue the run/step)

If a breakpoint has "no break" or "auto-continue" (the latter differs 
from the first), then parts of the frontend may need to know.
This can to some extend be managed in the frontend itself, by examining 
the current breakpoint(s) [1].
But it can't be entirely taken from the backend, because if the backends 
state is "mid single step", then the frontend can not sent a "step over".
If maintaining some of the state info is moved to the frontend, this 
must avoid duplicating info that is bound to the backend. (to avoid them 
becoming out of sync).

[1] Btw, off topic, there may be more than one breakpoint on the same 
address, so there needs to be a list of breakpoints hit...


===> Going forward

It might be best to look at an actual list of new states and events.

- What do different parts of the frontend need to react to (events)?
   And can/should the backend(s) know about each of those "causes".

- What states are important, from a frontend/users view.
- What mapping, between abilities and state do we need, and where to 
compute them.


For example the debugger already has
     function  GetCommands: TDBGCommands; virtual; // property Commands
And this returns what is currently possible
     if dcEvaluate in Commands then {watches can be evaluated}
- Except, that may be returning wrong for dsInternalPause => so there is 
some fixing needed.
- The naming is bad, because its more than just commands. Maybe should be:
    ~ dcEvaluate in CurrentCapacities
    ~ Debugger.Is[Currently]Capable(dcEvaluate)
    ~ dcEvaluate may be to narrow, if it includes ability to 
read/provide stack/thread/... Or maybe there can be separate enums.


As for events, here are the events that currently can trigger watches to 
be refreshed:

By the debugger (IIRC)
   WatchesNotification.OnUpdate    := @WatchUpdate;
     // send by the debugger  WITH watch=NIL, equal to the callback in 
your code
     // send by the debugger  WITH watch<>NIL, equal to 
edeEvalutionStateChanged
   ThreadsNotification.OnCurrent   := @ContextChanged;
   CallstackNotification.OnCurrent := @ContextChanged;

By the frontend itself
   WatchesNotification.OnAdd       := @WatchAdd;
   WatchesNotification.OnRemove    := @WatchRemove;
   SnapshotNotification.OnCurrent  := @SnapshotChanged;

WatchesNotification.OnUpdate could be split into 2 events.
It is also the same for start/end availability of data.
That may want to be extended. Though the info can already be retrieved 
from the debugger. (DebuggerState / Commands)

Yet it is not even needed to get the state. (Or should not, I have to 
check if it works).
The watchvalues validity should be enough.
If correct, it should be ddsInvalid.
If a "ddsEvalNotPossible" is added, then the info is avail (and the 
timeout can be set, depending on debuggerState)


The threads and callstack need to be separate notification, Different 
windows may subscribe to different sets of them.

So I am not sure, if indeed we need an entire new event system.
One could argue that thread and stack should trigger 
WatchesNotification.OnUpdate.
But that would be wrong.
=> I have often thought it would be good to have a 2nd (or more) watch 
window, and have a setting for that window to stay at a fixed 
thread/stack, even if the selection in the stack window is changed.
- So such a window, would not change if the current stack in the 
stackwindow was changed
- But it would need to reload, if the user changed the value of a watch, 
and therefore modified the target memory. (which could affect other 
watches too)

So in the end, the watches window needs to be able to decide which 
events it wants.




On 14/09/2021 11:00, Joost van der Sluis via lazarus wrote:
>
> I have something similar as the data-monitors.
>
> Main difference is that the events are based on what the gui needs, 
> instead of what the debugger does.
>
> So: there is an event to tell the 'gui' that it is inpossible to show 
> any data. (for example: the application has stopped or there is no 
> debugger at all) There is an event for the case that debug-data has 
> become available or unavailable. (Application paused or continued)

  We have an event, when watches can start to be evaled 
"WatchesNotification.OnUpdate".
See above.


>
> The advantage of 'events' that are more geared towards the gui, is 
> that it is easier to add catchy things. For example: to avoid 
> flickering, when the gui receives an event that debug-data is not 
> available anymore, it starts a timer of 200 mseconds, and only after 
> that timer the data is removed from the screen. This way, when 
> stepping through the code, there is no flickering in the small periods 
> of time that the application runs.
>
> It also made it easy to highlight variables that changed.
>
Both can be done, with the existing system. Though some optimizations 
can be applied.