Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discuss - There should only be one (Application.Top) #2502

Closed
tig opened this issue Apr 4, 2023 · 5 comments
Closed

Discuss - There should only be one (Application.Top) #2502

tig opened this issue Apr 4, 2023 · 5 comments
Labels
v2 For discussions, issues, etc... relavant for v2

Comments

@tig
Copy link
Collaborator

tig commented Apr 4, 2023

This is all debatable, but here's my thinking:

TopLevel is currently built around several concepts that are muddled and have led to overly complex code:

  • A Toplevel is a view that can host a Menu and StatusBar. Other views cannot.
  • A Toplevel can run modally, and have its own Runstate (via Application.Run(). It is unclear why ANY VIEW can't be run this way, but it seems to be a limitation of the current implementation.
  • A Toplevel can be moved by the user (actually only the Toplevel subclass, Window). It should be possible to enable the moving of any View (e.g. View.CanMove = true or similar).

Application is the place where all of the above TopLevel gunk happens in addition to other "application" stuff.

The MdiContainer stuff is complex, perhaps overly so, and is not actually used by anyone outside of the project. It's also misnamed because Terminal.Gui doesn't actually support "documents" nor does it have a full "MDI" system like Windows (did). It seems to represent features useful in overlapping Views, but it is super confusing on how this works, and the naming doesn't help. Application is full of code like if (MdiTop != null && top != MdiTop && top != Current && Current?.Running == false && !top.Running) which just screams poor-OO design and fragility. This all can be refactored to support specific scenarios and thus be simplified.

My proposal:

  • Application.Top should be set when an app starts, and should not be changeable. Changing it after Application.Begin has been called should result in an exception.

  • The Application class should do the following (and nothing else; other things should be moved elsewhere as appropriate):

    • Application-wide settings (like default quit key and diagnostics)
    • Initializing and configuring the console driver
    • Providing Runstate and managing the Mainloop
    • Providing a simple API for starting applications either with a default Top view or a developer-provided one
    • Proxying mouse/keyboard events from the driver to the Top
  • Stuff that will be moved out of (or deleted) from Application:

    • All the Mdi stuff
    • All the tracking of Current and toplevels. This stuff will just be part of View.
    • Code for moving from view to view in the focus or z-order heirarchies
  • All Views should support overlapping within a superview and have a menubar and statusbar.

  • All Views should support having subviews be overlapping

  • All the insane logic in Application.cs for dealing with Current, the stack of toplevels, etc, should be moved into the View hierarchy (either as part of View directly or a helper class)... and radically simplified. This includes getting rid of all of the Mdi stuff.

  • Any View can be run modally (as a popup that captures all user input) can be executed with a new API: view.Run(popupView) as is today, except they don't have to be of type Toplevel. When such a view is run, view.Modal is set to true letting the view know it's modal. We'll retain Application.Run() for backward compatibility, but it will simply do Application.Top.Run(popupView).

Making this happen requires the following to be done first:

But it seems doable, and will significantly simplify the programming model (and codebase).

What do folks think?

@tig tig added the v2 For discussions, issues, etc... relavant for v2 label Apr 4, 2023
@BDisp
Copy link
Collaborator

BDisp commented Apr 4, 2023

The only advantage from having multi toplevels in the Application is if we can open other console by a process or whatever and interchange arguments between them. In this case it can act as non singleton. Only an idea :-)

@tig
Copy link
Collaborator Author

tig commented Apr 5, 2023

The only advantage from having multi toplevels in the Application is if we can open other console by a process or whatever and interchange arguments between them. In this case it can act as non singleton. Only an idea :-)

I'm not understanding this. Can you explain further as it sounds interesting!

@BDisp
Copy link
Collaborator

BDisp commented Apr 5, 2023

I'm not understanding this. Can you explain further as it sounds interesting!

I'm only be vague :-)
Imagine you create an app with a ThreadApp attribute where if no arguments are passed you create an instance of the app in the current thread. If a argument is passed, like a toplevel object serialized by calling in a new thread using the Process class:

ProcessStartInfo startInfo = new ProcessStartInfo("myApp.exe") {
            Arguments = "myToplevelSerialized",
            UseShellExecute = true,
            CreateNoWindow = false
}
Process.Start(startInfo);

When the console is loaded with a argument the attribute will de-serialize the argument and run it by call Application.Run(myDeSerializedToplevel). You maybe can also run a Dialog in a new console and wait for exit.
But of course the toplevels opened on others console only will be added in their Application stack.

The Mdi can be achieved by adding other toplevels to the Application.Top and thus the toplevels stacks wont be needed. But was fun develop them :-)

@tig what do you mean by Terminal.Gui doesn't actually support "documents"? What do you mean with "documents"?

@BDisp
Copy link
Collaborator

BDisp commented Apr 5, 2023

@tig but thinking better we can maintain the Mdi property and MdiChilds to differentiate if all toplevels of the Application.Top are Application.Top's child or arbitrary toplevels added to the Application.Top. If Mdi is true then the Application.Top and the opened childs are notified by subscribed events. Otherwise, if Mdi is false all the Mdi related events aren't invoked. The main change is that child aren't added to the Application toplevels stack but added as sub-views of the Application.Top. What do you think about this?

@tig
Copy link
Collaborator Author

tig commented Jan 10, 2024

@tig tig closed this as completed Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 For discussions, issues, etc... relavant for v2
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

2 participants