-
Notifications
You must be signed in to change notification settings - Fork 700
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
Fixes #3692++ - Rearchitects drivers #3837
base: v2_develop
Are you sure you want to change the base?
Conversation
Hope you're ok with me updating the title & first post... |
@BDisp the
Gets returned as
This then breaks
For now I am using workaround: /// <summary>
/// Converts terminal raw input class <see cref="ConsoleKeyInfo"/> into
/// common Terminal.Gui event model for keypresses (<see cref="Key"/>)
/// </summary>
/// <param name="input"></param>
/// <returns></returns>
public static Key ConsoleKeyInfoToKey (ConsoleKeyInfo input)
{
ConsoleKeyInfo adjustedInput = EscSeqUtils.MapConsoleKeyInfo (input);
// TODO : EscSeqUtils.MapConsoleKeyInfo is wrong for e.g. '{' - it winds up clearing the Key
// So if the method nuked it then we should just work with the original.
if (adjustedInput.Key == ConsoleKey.None && input.Key != ConsoleKey.None)
{
return EscSeqUtils.MapKey (input);
}
return EscSeqUtils.MapKey (adjustedInput);
} |
… running (net or win)
Starting my review... Some things I noticed in pulling down and running locally. This is with "ConsoleDriverFacade(win)"
This stuff leads me to believe this isn't really ready for merge, right? But you want me to review the code? |
Running UICatalog "All Views Tester" -b -t 5000 --driver v2net Shows lots of issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments here.
My review was pretty superficial. I didn't actually dive deep into the tests.
What is your thinking on how we move forward with this?
It appears that unless you explicitly enable the new drivers, everything works swell.
Is the intent to merge with the new drivers "broken" and then fix them off of v2_develop?
I would like to fix bugs. Especially any crash bugs. Biggest issue I noticed was with menus. They seem to be 'special case' for Top and have explicit code in Application that interacts with them. I was hoping that if we could get it so any view can have menus and they work just like any other subview - then the issues might become less opaque and/or disappear. Let me try the all views tester. I have run many scenarios in isolation but it's unclear what some of them are meant to do upon cursory examinations. It would be great to have help identifying the main outstanding 'show stopper bugs' in terms of behaviour. |
I want to solve all the 'driver level' bugs but then address 'view specific' issues later e.g. HexView always reporting that it NeedsLayout/NeedsRedraw constantly true. The new 'main loop' is more trusting of the views reporting their state. For example there are no longer any 'out of band' screen Refresh allowed. Console is only flushed/updated after loop iteration and only once (and only if a View was needs draw true). But for that to work properly the views have to behave. The logging helps identify which views are misbehaving in this regard. |
Looks like AllView tester is double booting. And the LayoutEditor is constantly reporting true for redraw. Probably this should be an Exception though or ignored not bad behaviour
|
Hence the Issues and PRs around Top and rewriting Menu. See those.
Yes. See related menu PRs. |
- Ignore double init - Raise init changed event
Colors fixed, I just wasn't calling the init on config manager from the new bootstrapping Application2 class init |
private void HackLayoutDrawIfTopChanged () | ||
{ | ||
// TODO: This fixes closing a modal not making its host (below) refresh | ||
// until you click. This should not be the job of the main loop! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. That must be an Application
job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow, it seems to work with the original drivers but not the v2 ones.
V2 drivers only listen to these properties (needs layout/draw). There are no manual or triggered draws, so possibly that is why this hack is needed for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but I think that if you do the same thing in the Application.LayoutAndDraw
method you'll will obtain the same result, but I could be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my code, layout and draw is only called if a subview of Top wants redrawn or layed out... I will look into it some more later
This is a super set of #3791 and exists to see diff and direction of v2 drivers as discussed in #3821
Fixes
ConsoleDriver
to minimize duplicated code in all drivers #2319EscSeqUtils
#2803ConsoleDriver
s to support ANSI sequences natively (e.g. Virtual Terminal Sequences on Windows) #2610Button1Clicked
events onNetdriver
orCursesDriver
#3374Metrics
You can now see metrics on ant Terminal.Gui program by using its exe name and a metrics tool e.g.
Implementation uses System.Diagnostics.Metrics making it integrate easily with tools such as OpenTelemetry
Logging
You can enable logging of internals of Terminal.Gui by setting
Logging.Logger
to anyMicrosoft.Extensions.Logging
compatible logging framework (nlog, serilog etc).Naturally you must not use a console logger
For example with Serilog to a file:
Progress
MouseInterpreter
)Proposed Changes/Todos
Splits drivers up into logical components, simplifies and centralizes shared code patterns.
Pull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commit///
style comments)