MR - DebuggerIntf for terminal window

[MattBradford]

First part first, since I wrote it badly.

if it proposes, to change how the desktop code (or the IDE window placement
in general) works

It did, and I withdraw it. A slot-aware layout store, and suppressing windows
during desktop restore, are both changes inside code you have just told me has no
active maintainer. Not worth holding up a console API to go and find one.

Here is a version that needs nothing new.


Selection decides whether the window can be created at all

Both restore paths already funnel through IDEWindowCreators.GetForm, and both
already cope with getting nil back.

Undocked: RestoreSimpleLayout calls GetForm for each stored layout. GetForm exits
nil with "no creator for X" when nothing is registered under that name, and the
loop does "if AForm=nil then Continue".

Docked: FDockedOpt.RestoreDesktop goes into AnchorDocking, CreateControlsForNode
calls DoCreateControl, which logs "control not found", returns nil, and the node
is skipped. OnCreateControl there is DockMasterCreateControl, which is GetForm
again. Missing controls are already routine on that path — an uninstalled package
produces exactly the same thing.

So: a provider's window can only come into existence while that provider is
selected. The least invasive form does not touch the creator registration at all.
Keep the creator registered, and have its OnCreateFormProc return nil while the
provider is not active. GetForm logs "create failed" and returns nil, and the
restore skips it as above. Nothing to unregister, and no registration ordering to
get wrong.

Your F9 sequence then reads: the desktop asks for IdeDbgConsole, the built-in is
not the selected provider, no form is created, and the stale window never appears.
The provider the user did select opens from its own layout entry as usual.

The cost is two debugln lines per suppressed window, on a path that already logs
the same for uninstalled packages. If you would rather this case stayed silent,
that is the only piece that would want a change in shared code, and it is
cosmetic.


What this does not give you

Being straight about the one thing that is worse than what I proposed before.

TSimpleWindowLayout.ReadCurrentState sets Visible := (Form<>nil) and
Form.IsVisible, while ReadCurrentCoordinates returns early when Form is nil. So a
deselected provider keeps its stored geometry, but has Visible flipped to false
the next time window positions are stored.

The effect: use A, switch to B, save the desktop, switch back to A, restore — A's
window does not reopen. The user opens it once from the menu and it behaves from
then on. Position is never lost, and it only arises for a desktop saved while a
different provider was active.

I think that is acceptable, and better than modifying layout storage nobody is
maintaining. If you disagree, the next thing I would try is having the provider
open its own window when it becomes active rather than leaving it to the desktop,
which is again entirely in package code.

That also closes the question I left open earlier, about whether an incoming
window inherits the outgoing one's position. It does not: each provider keeps its
own remembered layout under its own window id, which the existing machinery gives
us for free. It is what I already have, so there is nothing to build.


Naming

PluginId, case-insensitive compare, third part allowed and no fourth — agreed on
all of it. The name-stealing argument for case-insensitive is a better reason than
the one I gave.

dtcUnknown, dtcStdOut, dtcStdErr — agreed, and thanks for the correction. I
reached for scoped enums out of habit, and consistency with the surrounding code
matters more than my habits.


Supports

maybe instead of implementing them now "just in case", doing a forward
compatible interface would be smarter

You caught a real mistake in mine: class level on an interface needs an instance,
so it belongs on the registration class. Given that, and given your second
thought, I would take the forward-compatible route and not add it now. I have no
case today that needs it — my provider is supported wherever it compiles. Adding
it later to the registration class with a default, or to the interface through the
template, will cost whoever needs it less than the two of us guessing the values
now.

The part worth keeping for when it is added is your point about returning a reason
string. "Not available", with no explanation of why, is the kind of thing that
generates forum posts.

[Martin_fr]

So: a provider's window can only come into existence while that provider is
selected.

Generally ok.  Also probably easiest, and easiest to adapt should later new valid use cases arise.

Undocked: RestoreSimpleLayout calls GetForm for each stored layout. GetForm exits
nil with "no creator for X" when nothing is registered under that name,

But if the form is already show on screen, then it will be gotten from there.

Code: Pascal  [Select][+][-]

1. function TIDEWindowCreatorList.GetForm
2. ...
3. for i:=0 to Screen.CustomFormCount-1 do

Apart from that (if the above can be solved):
We need to define (more an issue of definition, rather than technical) when that selection should be updated.

Throwing in random thought at first... Sorting out below.

Destkops can change (at least)
- when switching between debug/non-debug  (maybe other modes in future)
- when a user manually selects it
- when projects are loaded (I think?)
- when the IDE starts => that one is crucial, because we don't want loading order dependencies if anyhow avoidable

The selection changes when
- the run params ares changed or edited.
- when the debugger backend is changed (and an entry becomes unavailable)

1) Should changing the run mode trigger a change of open window (as in immediately)?
Or should that be deferred to: when debugging next times starts
Or alternatively, when the user changes the desktop (if they have many stored)

I have an inkling to say that picking a "run params" mode from the drop down in the run-button, should not immediately close/open windows. Especially since you can easily pick the wrong one, and pick again.

If it doesn't change immediately, then the "current selected" is left at whatever it is, until some trigger event updates it. That matter below.


2) Desktop changes (not triggered by start debugging).
Should they make the change?
May be a matter of consistency:
- If the entirely close that window, then they do that (no question)
- If they open it (from previously closed), then simplicity dictates that they should only be able to open the current selected.  But is this a trigger to update the current selected?
- If the window is open, and kept open => should that trigger the update ?

Obviously starting debug must trigger the update.

There is the first question, can that easily be detected, before it is needed for decisions in changing the layout?

If not, or if for other reasons, then any "desktop switch" is an update trigger.
And again, would be the simplest.

Back to "GetForm" and Screen.

That would work, if the form would switch (close current/open other) immediately when the "run params" change. Because that wouldn't be a "desktop change". That would just be an event, upon which the debugger could close or open windows as its pleased.

Of course, the same event would also work, if it was a delayed trigger.
- Switching to debug: If the debugger closes any "not available" window before the desktop switch happens, then the desktop just shows the new window, and does get nil for any other => works
- Other events (such as manual desktop change): would work, if we can get a notification before it happens.

In any case, only close "not available". Opening of the new is done by the desktop change.

So that may be solve-able, but a bit more work than anticipated.

And then its all about the above questions.

TSimpleWindowLayout.ReadCurrentState sets Visible := (Form<>nil) and
Form.IsVisible, while ReadCurrentCoordinates returns early when Form is nil. So a
deselected provider keeps its stored geometry, but has Visible flipped to false
the next time window positions are stored.

Indeed an issue for autosaving desktops. But for now, it has to be accepted.

The rest seems to be agreed. Good.

[MattBradford]

But if the form is already show on screen, then it will be gotten from
there.

You are right, and I missed it. GetForm scans Screen.CustomForms before it ever
looks at the creator list, so returning nil from the creator only suppresses a
window that does not already exist. Thank you for catching it.

Worse than it first looks, and worth stating plainly: the LCL adds a form to
Screen in the constructor and removes it in the destructor, and that scan has no
visibility test. So hiding is not enough either — Close leaves the form on the
list, GetForm hands it back, and Apply/ShowForm puts it on screen again. The
provider has to destroy its window, not close it. "Only close not available" has
to mean free.

With that, I went looking for whether the "before the desktop switch" timing you
described is actually available. Two of the three cases already have it.


Starting debug: already ordered correctly

TDebugManager.OnDebuggerChangeState calls the state notification list first, and
only afterwards UpdateButtonsAndMenuItems and UpdateToolStatus. UpdateToolStatus
is what maps the state to itDebugger, which sets MainIDE.ToolStatus, which calls
EnableDebugDesktop, which calls UseDesktop and then RestoreDesktop.

So a debugger state-change subscriber runs before the desktop switch, in the same
call. That is the hook you were already planning to add for state changes, and it
lands in the right place with no extra work: the provider frees its window on
dsInit, and the restore that follows finds nothing on screen. It is also where the
existing console does its Clear, so the precedent is there.


IDE startup: also already there

TMainIDE.RestoreIDEWindows calls the lihtIDERestoreWindows handlers and then
EnvironmentGuiOpts.Desktop.RestoreDesktop. Since that is a single pass after the
packages have registered, a provider hooking
LazarusIDE.AddHandlerOnIDERestoreWindows can reconcile once, before anything is
restored, with no dependency on load order. That was the case you called crucial,
and I think it is covered.


Manual desktop switch: the one gap

All switches funnel through TEnvGuiOptions.UseDesktop — the desktop manager for a
manual pick, and EnableDebugDesktop / DisableDebugDesktop for the debug ones. One
choke point, which is the good news.

The bad news is that nothing there announces itself in advance except
EnvironmentOptions.DoBeforeWrite on the first line. That is hookable through
AddHandlerBeforeWrite in ideoptionsintf, but it is the wrong signal — the IDE
options dialog and the package editor call DoBeforeWrite too, so a provider
hooking it would tear its window down whenever anyone wrote options.

What would close the gap is a handler list on UseDesktop, fired before
Desktop.RestoreDesktop. That is additive and touches no layout storage and no
placement logic, so I do not think it runs into the maintenance problem you
raised — but it is IDE code and therefore your call, not mine. If you would rather
not, the manual-switch case degrades to the same "user opens it once from the
menu" that the Visible flag already gives us.

One thing off your list, incidentally: I can find no path where loading a project
switches desktops. The only callers of UseDesktop are the desktop manager and the
two debug ones.


Now the questions you actually asked

1) Should picking a run-params mode from the dropdown immediately close and open
windows?

No, and for your reason: it is easy to pick the wrong one and pick again, and
windows appearing and disappearing under that is unpleasant. I would define the
stored selection as "what the next debug session will use", and let it sit until a
trigger.

2) Should a desktop change update the selection?

Yes, and I would make it unconditional rather than reasoning per case. Your three
sub-cases — desktop closes the window, desktop opens it from closed, window open
and stays open — each have a defensible answer, but a rule that depends on what
the desktop happens to contain is hard for a user to predict and harder to test.
Uniform is better: on any desktop switch, reconcile first (free anything not
available), then let the restore open whatever the desktop asks for. You said
simplest, and I think simplest is also the most explicable.

Starting debug obviously triggers it, and per the above it already can.

So the whole rule is: selection changes are stored but not acted on; reconcile
happens at IDE startup, at any desktop switch, and at dsInit. Three triggers, all
of them "something is about to lay out windows anyway".

[Martin_fr]

1) Should picking a run-params mode from the dropdown immediately close and open
windows?
No, and for your reason:

2) Should a desktop change update the selection?

Yes, and I would make it unconditional rather than reasoning per case.

Generally ok...
Though, once in the IDE it may trigger feedback and requests.

The two issue I see (but don't have an answer how to make better) are:
- closing before is the only options. It may lead to flicker (and hopefully anchordock wont show any issues with it / it still has its own issues)
- switching a Destkop forth and back wont do a restore: your are on desktop A (and show the LazCons), you switch run mode to TermCons, but still see LazCons, you switch Desktop forth and back.

But as I said, I don't have anything better.

-------------------------
In any case make that either 2 MR, or have all the commits for switching separate and at the end please.


I don't have personal usage experience with that. So I can't predict what happens.

But for example for a long time (now there is a better alternative), I used the power buttons on the watches to keep the display frozen. (and then used hints instead to get new value).
I even used that to have the values after the debug session ended.

That last bit is something that may matter. You did a debug run, you have the output of your app, you work on your app to adjust what it prints. So that output must stay on the screen.

If you change desktop, users may want that to still be possible.
- if the provider hasn't changed, all is good. Nothing special is done
- if it changes to another provider (already set for the next session) then the user wont see the window anymore

I don't know but some people may even want to use that to have 2 independent displays, one with the unchanged output of the previous run, and one with the current. Despite that is not the aim of the providers.

Now those assumption on what people may want may be totally wrong... I don't know. But I have seen a good deal of user feature requests way outside of my range of imagination.  (you probably noticed that I tend to try and thing of many corner cases)

So more important than the current behaviour, is how easy we later can add other behaviours on top.


Conclusion:
- go with "change on each desktop switch", if that works
- keep the work needed simple / lets treat it as getting feedback and experience on how it feels in everyday work.

[MattBradford]

Understood on all three, and noted on the MR shape — the switching work will
either be its own MR or sit at the end as separate commits.

On output surviving a run: that one matters to me too, and it is already how
mine behaves. The window outlives the stopped process deliberately, unlike the
OS console which goes away with it — you read what your program printed while
you edit the program that printed it. So rather than freeing on deselection, a
deselected provider will stand down but keep its window, and only free when a
session is about to start. Same reconcile points, one step later. That keeps
your two-displays case reachable later instead of designing it out.

One thing I should have asked several posts ago.

Who writes the unit in IdeIntf?

The generic plug-in list is yours, that has been clear since #6 and I am not
proposing to write it. But the console-specific registration entry and the two
interfaces are what your #8 and #14 drafts describe, and I do not know whether
you intend to land that unit yourself, or whether my MR should contain it built
to those drafts.

I am happy either way. If it is yours, I will build the provider against it once
it is in and there is nothing for me to do until then. If you would rather I
wrote it to your draft and you reviewed it in the MR, I will do that instead —
it is your API either way and I would follow the draft rather than reinterpret
it.

I ask only because I would otherwise either sit waiting for a unit you were
expecting from me, or hand you code you had already written.

[Martin_fr]

The generic plug-in list is yours, that has been clear since #6 and I am not

I am a bit busy right now, so to not hold you back, and if ok with you:

You start, don't worry about making it a generic, just but in your class for the list.
I can then (either when the MR arrives, or later) make the changes to turn it into generic.

Equally, I can fix any names, or rename units if that is needed (and if you don't take that as critic on your work).


Would that work for you?

[MattBradford]

That works, and it is the answer that unblocks me — thank you.

I will write the list class concrete rather than generic, following the drafts
in #8 and #14, and leave the generalising to you. Names and unit names likewise:
I will use what we have agreed here, and anything you rename afterwards is yours
to rename. It is your API, and I would rather you changed it than lived with a
name you did not pick.

The switching work will be separate commits at the end, or its own MR, as you
asked.

[MattBradford]

I have the registration, the chooser and the reconcile working, with a second
console window installed as an ordinary package alongside the built-in. It
switches, it takes settings, and output and input both go where they should.

Then I took it near a docked IDE with two desktops, and ran into something I
cannot solve from a plug-in. It is one problem wearing three faces, so let me
give you the faces first.

- Switch console window: the outgoing one loses its docked position.
- Switch back again: it does not return to where it was either.
- Switch desktops a few times: the two desktops converge on the same layout.

The cause

Suppressing the outgoing window means it must not exist. A window that is
merely closed is still found -- GetForm scans Screen.CustomForms before it
consults the creators, with no visibility test -- so a desktop whose entry says
"visible" shows it again, which is your F9 case.

But destroying is exactly what the layout machinery is not expecting.

TSimpleWindowLayout captures coordinates in one place only:

Code: Pascal  [Select][+][-]

1. procedure TSimpleWindowLayout.OnFormClose(...);
2. begin
3.   GetCurrentPosition;
4. end;
5.  

Destruction goes through Notification/opRemove instead, which sets fForm to nil
and Applied to false and reads nothing. So a destroyed window has no position
stored.

Closing before freeing fixes that much, and the undocked case then behaves.
Docked does not, because AnchorDocking records the site in one place only too:

Code: Pascal  [Select][+][-]

1. DockMaster.RestoreLayouts.Add(DockMaster.CreateRestoreLayout(HeaderParent),true);
2. HeaderParent.CloseSite;
3.  

That is the header close button. A control that vanishes from under the dock
master never gets a restore layout, so nothing remembers where it was. I can
see no way to reach that from a plug-in without depending on AnchorDocking,
which a console window has no business doing.

The third face is the same thing meeting the desktop code. Every switch runs
TDesktopOpt.ImportSettingsFromIDE on the desktop being left:

Code: Pascal  [Select][+][-]

1. IDEWindowIntf.IDEWindowCreators.SimpleLayoutStorage.StoreWindowPositions;
2. ...
3. FIDEWindowCreatorsLayoutList.CopyItemsFrom(IDEWindowIntf.IDEWindowCreators.SimpleLayoutStorage);
4.  

The reconcile at dsInit takes the window down, and then ToolStatus becomes
itDebugger and EnableDebugDesktop snapshots a layout in which that window has
Form=nil -- so Visible is false and the coordinates are stale. Do that in both
directions and each desktop picks up the other's degraded entry.

The choice

With the code as it stands I can have one of these, not both:

1. Destroy the deselected window. Nothing stale is ever resurrected, and the
   F9 case you raised is solved. Dock position is lost on every switch, and
   desktops degrade as above.

2. Close it and leave it alive. Layout and dock site survive. A saved desktop
   whose entry has that window visible will show it again, receiving nothing.

If it is my call I would take (2), and it is also what you actually asked for
-- "in any case, only close 'not available'". Losing the dock position on every
switch is a daily irritation, while the resurrection needs a saved desktop that
had the other console open, and sits in the same category as the Visible flag
on autosave that you have already accepted for now. It leaves the layout
machinery untouched, which given no one is maintaining it seems the right
instinct.

I should say plainly that this reverses what I argued last time. I said
destroying was necessary and I was right about why, but I had not then found
what it costs.

What would make (1) workable, if you would rather have it

Something that means "remember where this window was, I am taking it away",
reachable without knowing which dock master is installed. TIDEDockMaster has
ShowForm and CloseAll and no equivalent. If such a hook existed the plug-in
would call it before freeing, and both problems above would go.

That is a change to window management rather than to the debugger, so I am not
proposing to write it, and I am not asking for it unless you think it is worth
having for other windows too.

Not blocking me

Everything else is done and testable, docked and undocked: registration,
selection, per-plug-in settings with the frame rendered by the IDE, the menu
entry opening the selected window, and the reconcile at startup and at dsInit.
I will follow whichever of the two you pick -- it is a one-line change in the
plug-in either way, and no change at all to the interface.

[Martin_fr]

I kind of agree on 2 or maybe a step further even.

Anything that risks upsetting the desktop managing or anchordocking has to be saved for another day (hopefully one that will come, but...)


Given that
1) many people will set up a provider for their project, and keep that => they can setup a desktop to match it (burdens the user, but once off)
2) people even may even want to have both providers open (to see one as history)

I would just leave the windows open.

The provider gets told it has been removed => It can add a note to the window that it is inactive. So users will not be fooled by its presence (e.g. add "disabled" to the caption)

The menu can still be re-assigned, it opens or brings to front the current active provider's window.

To make it consistent, a sub-menu could list all providers, and so the user would have access to those too, if they wanted.

IMHO
- its consistent
- it leaves the user in charge
- it doesn't do anything new as far as desktop and docking go

Question: How about your workflow. Do you have any case that aren't covered by this, or that become less convenient?