Terminal.Gui
Terminal.Gui copied to clipboard
gui-cs
Refactoring, cleanup, and abstraction for ConsoleDrivers
Filing an actual issue now that I've gotten to work on things that have been talked about here and there and some discretionary/opportunistic bits along the way.
This one starts with the console drivers.[^NoBullyingTheDrivers]
I'm nearly done with what I'm slapping a round 1 label on and will have a PR ready later today, hopefully.
Things that are addressed so far:
- General code cleanup and organization.
- Bug fixes
- Addressing both theoretically possible bugs based on the intent and usage of the code as well as possible problems pointed out by static analysis, such as potential null reference exceptions.
- Tweaks to certain exception throwing and handling to help identify, report, and handle what's going on and to align exception handling closer to design guidelines and conventions.
- Preventing the conditions that lead to certain avoidable exceptions.
- Type-safety/correctness
- Things like generics, mostly.
- Also cases related to nullable structs, null checks, polymorphism, and native interop.
- Thread-safety
- That part (so far) is minor and mostly informational - comments and such.
- More specific concurrency work will happen later on, whether here or in a separate issue.
- Performance
- Various approaches to try to make things a bit leaner where I see a reasonable opportunity to do so.
- Things like ref passing, use of ref locals, use of Spans, avoiding run-time values that are still just constants and making them compile-time constants, etc.
- Priorities are in this order:
- Speed and simplicity, with basically equal priority
- Simplicity - I'm not going to do things that are hard to maintain or understand.
- Execution speed - Largely avoiding copies and avoidable heap operations.
- Memory consumption
- Compilation, design-time, and testing performance
- Small but non-zero work items to reduce how heavy the solution is on dev and CI machines, overall.
- Compiled assembly size
- Not really directly addressing (at least not yet anyway), but paying attention to it.
- Speed and simplicity, with basically equal priority
- Trim-friendliness
- Eliminating uses of reflection.
- Use of generics instead of
Typevalues.- This one also falls under performance, and is a type-safety thing, too. Type-typed method parameters are a break in the chain for static type analysis, which is why they aren't trim-friendly.
- Nullability context
- Turning it on wholesale for most files I'm touching, and in targeted sections in others I'm not interested in fully migrating in this round.
- Also using the resulting analysis to improve code in various places where it makes or could make a difference.
- This also falls under bug fixes and type safety.
- Turning it on wholesale for most files I'm touching, and in targeted sections in others I'm not interested in fully migrating in this round.
- Interop/PInvoke
- Migration to source-generated interop.
- Using CsWin32 to create more robust native interop types and methods.
- Some or potentially all generated code may get copied to permanent files since the APIs used are stable, to enable hand-tweaking and for sake of transparency and simpler debugging.
- Some structural reorganization
- Moved some files.
- Split multi-type files that were worked on into one type per file.
- Testing improvements
- More parameterization
- DRY-er code (largely enabled by generics and things like
MemberData) - Use of Assume (from the Xunit.Assume extension package) to differentiate pre-conditions in some tests from the test itself (my god why is Xunit so terrible???).
- Better determinism/repeatability.
- Removing some uses of CombinatorialRandom, since that makes every test run different where it is used.
- Using and re-using new common helper methods to provide simple, clean, and consistent generation of test cases.
- Applying attributes such as category traits to various tests and test classes.
- Tweaking some tests to provide better accuracy, speed, simplicity, etc
- Probably more, but I want to get back to work now.
I've done it as a ton of mostly smaller commits, as usual, to make it easier to follow along through the commit graph. However, one of the biggest changes I made at the start is actually what prompted some of the additional work that got done in this round, and hasn't been committed to the branch yet, so the current head of https://github.com/dodexahedron/Terminal.Gui/tree/v2_d12_some_driver_cleanup_1 will not compile at the moment, though my local copy (which of course DOES have those changes) compiles and passes all tests. I just need to finish the work on it before I can commit and push that part.
PR will come when I push the commits with that work finished.
There are also a few extra bits of work I want to do before round 1 is done, beyond that, but those will come after a draft PR is submitted.
[^NoBullyingTheDrivers]: No specific reason about the code or code quality itself for picking that as a starting point for this work. It was based on the fact that they're alphabetically early in the project folder tree, they're some of the first code that runs, and the first warnings in the list when I turned analysis up a notch locally brought me to one of those files.
Oh another biggie I forgot to stick in the list of stuff that's already (mostly) done, and which falls under more than one of those top-level categories:
The code around use of unmanaged resources like the console file handles used with SetConsoleMode and such has been reworked to use actual SafeHandles (FileHandles, primarily) to enable better/safer/easier use of them, including proper lifetime management/disposal.
That's important because Terminal.Gui's lifetime is in no way tied to the lifetime of the entire application (which is another reason I am particularly against unnecessary use of statics).
As a real-world case of that concept:
My SnapsInAZfs application, which led to me using and then ultimately contributing to TG, ONLY uses TG in a specific scenario, and otherwise the application typically runs as a headless service process or a one-shot process that has no need for a TUI. It's only used for a configuration interface.
Even unit tests expose some of the pain points there, which we've mostly worked around in other ways, but still don't fully address because we are implicitly taking advantage of the fact that test runner processes mostly do have TG lifetime (almost) equal to process lifetime.
The point is that we can't treat TG like it is going to be in use from start to end of the application. That makes proper handling of things like unmanaged resources pretty critical to not make any assumptions around.
@tig, a win32 question for you:
Have you ever tried or considered creating additional handles to the stdin/stdout/stderr file descriptors at startup time?
That method can be used to grab our own copy of the original file descriptors, which can then be used for the lifetime of TG, regardless of what the consuming application does.
Why do that?
It would fix cases where one or more of those are redirected after the application is launched, which currently causes a crash.
@tig, a unicode output question for you, RE: the NetWinVTConsole class:
It has the same guard against non-BMP runes as the CursesDriver does. Why is that there?
The windows console can display any UTF-8 character your font has a symbol for, including surrogates.
Behold:
// This code:
Console.OutputEncoding = Encoding.UTF8;
ReadOnlySpan<byte> bytes = "👨🏻👨🏼👨🏽👨🏾👨🏿"u8;
using Stream stdout = Console.OpenStandardOutput();
stdout.Write(bytes);
stdout.Flush();
// Has this output:
Also works fine for UTF-16:
Console.OutputEncoding = Encoding.UTF8;
ReadOnlySpan<byte> bytes = "👨🏻👨🏼👨🏽👨🏾👨🏿"u8;
using Stream stdout = Console.OpenStandardOutput();
stdout.Write(bytes);
stdout.Flush();
Console.OutputEncoding = Encoding.Unicode;
Console.WriteLine();
Console.WriteLine();
Console.WriteLine(Encoding.UTF8.GetString(bytes)); // Conversion to string is UTF-16. The UTF8 here is the SOURCE encoding.
For reference, the UTF-8 version of the string is 44 bytes, and the UTF-16 version is 20 chars (40 bytes), in case there's any question of whether those 5 emoji are multi-byte/surrogates or not.
There are a ton of nuances including versions of WT. it's possible they fixed Atlas since I last worked on this.
Happy to have it finally fixed. But I'm skeptical.
If you think you have a fix please test with CharMap.
Yeah I suspected that might be the sort of sticking point.
Mainly, I wanted to either be able to remove the IsBmp check on that or put a remark on it like the one in CursesDriver, for now, since it's currently without comment in that driver. This work isn't aimed at changing our unicode handling. That, if/when it comes, will be in a completely different change set.
We do have ways of continuing to drastically reduce allocations and copying and all that around handling of text, but that will also come in time and requires a lot more fine-grained testing than the sort of changes I intend for this and subsequent change sets in the series.
Update:
I Decided to go all the way with the PInvoke improvements for the files being touched in this round.
I let CsWin32 generate the initial interop types, and then I took the generated code and refined it to what you'll see in the PR. I then removed CsWin32 since further benefit from it only really exists if adding new PInvokes, specifically for windows APIs, so may as well not make builds take the extra couple of seconds. 🤷♂️
These involve no run-time code generation and only use blittable types for the platform calls, so are thus theoretically fully trim-safe.
I've still got a bit more left to do with it, but I'm working on it right now.
Update:
I Decided to go all the way with the PInvoke improvements for the files being touched in this round.
I let CsWin32 generate the initial interop types, and then I took the generated code and refined it to what you'll see in the PR. I then removed CsWin32 since further benefit from it only really exists if adding new PInvokes, specifically for windows APIs, so may as well not make builds take the extra couple of seconds. 🤷♂️
These involve no run-time code generation and only use blittable types for the platform calls, so are thus theoretically fully trim-safe.
I've still got a bit more left to do with it, but I'm working on it right now.
Huge.
I started doing this way back in #2490 but ran out of steam. You doing this will help that effort immensely!
Well, and, as part of it, I compared code generated by CsWin32 against the code[^notAFish] generated by use of LibraryImportAttribute.
CsWin32 tends to generate leaner code in some cases, but it also doesn't really understand some nuances of the underlying APIs, it seems, or at least for whatever reason does not generate quite as robust code around the marshaling of things between managed and unmanaged as LibraryImport does.
They are both source generators targeted at broad usage and compatibility, so of course they both generate slightly sub-optimal code in their own ways.
So what I'm doing is taking cues from the output of each, paring it down to what's actually relevant, cleaning things up to be legible, and whatever other tweaks make sense for these specific API calls and our use of them.
I'm pretty sure I mentioned a long time ago, in another pinvoke-related issue, that LibraryImport doesn't actually fully replace DllImport. DllImport is the mechanism used for PInvoke, at the end of it all, even for source-generated interop. It's just that, if you use DllImport without ensuring that everything has already been marshalled correctly and everything is blittable, the compiler ALSO emits a stub into the IL that triggers run-time generation of the code necessary to do all of that, and that's what is incompatible with trimming, because that's all Reflection.Emit and such.
One thing CsWin32 does that looks, on the surface, like it might be a nice advantage, is that is also generates structs to match native types like HANDLE. Cool, right? Well, yes, in some cases. But not this one.
There are already built-in types specifically for this stuff. In particular, SafeFileHandle and its associated types are much more robust way of wrapping the handles involved. The most basic operations they perform are pretty much the same - wrapping a nint and making them more meaningful than a plain nint. But SafeFileHandle also addresses some subtle but very real concerns, such as stronger reference counting to ensure that unfortunate timing of the garbage collector doing a collection don't result in things getting lost.
It also implements IDisposable and proper finalization, which isn't possible with a struct, because they do not participate in anything related to the GC (the other definition of "unmanaged").
However, CsWin32 did generate some helpful things, like enums for the various DWORD constants used for methods like SetConsoleMode and for the values of the STD_HANDLE constants for stdout, stdin, and stderr. Those things are handy, but aren't generated automatically by LibraryImport, since it only uses whatever types you specified in the partial method signature.
So, I'm keeping those, since they're simpler and more expressive than having a bunch of constants defined, and easier to use when calling things.
For any interop type added, I went to the documentation for those APIs and grabbed the relevant portions to stick into XmlDoc comments so we have a nice and fully-documented set of stuff to work with, without having to constantly go back and reference MS Learn.
CsWin32 makes an attempt to do that, but what it creates is REALLY messy and is just raw markdown taken from the github source for the MS Learn documents, which of course doesn't work well in XmlDoc comments.
So, it's been a little bit of a roller coaster with things being introduced, removed, re-introduced, massaged, etc., so I'm distilling the commits down to a lot fewer, mostly just dropping some that ended up being irrelevant noise in the end.
Another bonus to SafeFileHandle: it is cross-platform. .net has platform-specific code to make it work consistently regardless of operating system. That also opens up possibilities for more code reuse across the different drivers, since that is an abstraction that can then be pulled up to a less-derived type in the hierarchy - potentially even all the way to the base ConsoleDriver class. I am not doing any of that in this change set, however. This is just laying the groundwork.
[^notAFish]: Edited to fix a typo to avoid confusion between C# and aquatic animals.
OK, sooo...
I had a thought based on the win32 API calls used to interact with the console.
So yes, there's the handle. But...Why is it a SafeFileHandle? Because it IS a file handle, and always has been since the DOS days.
The device "files" (same concept as how unix treats it) are streams named CONIN$, CONOUT$, and CONERR$, and when you call GetStdHandle, that then just allocates another handle to point to the current process' stdin, stdout, and stderr streams.
And it does it via CreateFile. And the documentation explicitly says that's a way for you to grab a handle to each.
So I figured ok... There are already .net implementations of all of those APIs. It's only the Set/GetConsoleMode that don't exist as far as I can tell (as far as our usage of the console APIs goes, that is).
So I tried it out. And it works beautifully and also provides a way for us to fix a bug that we have basically just tried to work around by dealing with excptions, in the past: the problem of someone redirecting the console streams.
If you grab a new handle to the desired stream via File.Create(consoleDeviceName) where consoleDeviceName is one of the earlier mentioned special file names, you have a handle that the application can't rip out from under you.
Since doing things that require PInvokes that need handles to console streams appears to be a common question about, around the internet, I figured I'd write it up in a more public place than here, with explanation and code sample. The only answers I saw anywhere always immediately resorted to PInvoke, which tends to lead to at several PInvokes, which this method avoids a few of.
It is provided as this answer on Stack Overflow.
It's going to cut lines of code in this PR down by a fair bit. And any chance to avoid crossing the managed/unmanaged barrier in non-native code (ie our code) is a win, as well.
Continuing the above...
The same concept is valid on unix as well. After all, in unix-land, everything is a file - files, sockets, devices, and even you are all files.
The console for the current process is /dev/tty, and it is a bidirectional character device, meaning you only need one FileStream to both read and write to it.
If someone redirects stdin or stdout in the program, your FileStream you opened to it is unaffected, just like in Windows.
Unix systems typically also expose sockets that are essentially just symlinks to the console tty device, but in testing among different distros, versions, and shells, the specifics of that vary wildly enough to not be very attractive to try to figure out, though a "socket" abstraction is still basically the same idea, since it's still just a file like everything else. 🤷♂️
And of course they're fully capable of ANSI sequences, just like the original stream from the static methods on Console:
using FileStream consoleOut = File.Open ("/dev/tty", FileMode.Open, FileAccess.ReadWrite, FileShare.ReadWrite);
consoleOut.Write ("\x1b[38;5;190m\x1b[48;5;4mI'm writing colored UTF8 directly to the console!\x1b[39;49m\n"u8);
outputs:
The above was run on an Ubuntu 24.04 system, via ssh, connected from Windows Terminal 1.20.11781.0 on Windows 11 24H2 26100.1457.
Side note: Man, I wish they'd back-port the \e for escape to .net 8. 😅
Something that might make sense for us to do to enable easy switching to that in future versions is to define a constant like const char ESC = '\x1b'; and use it instead of \x1b in strings.
If put into a conditional as follows, it would automatically happen if compiled under .net 9 at least:
#if NET9_0_OR_GREATER
const char ESC = '\e';
#else
const char ESC = '\x1b';
#endif
const string ESEQ = "${ESC}[";
Side note: Man, I wish they'd back-port the
\efor escape to .net 8. 😅Something that might make sense for us to do to enable easy switching to that in future versions is to define a constant like
const char ESC = '\x1b';and use it instead of\x1bin strings.If put into a conditional as follows, it would automatically happen if compiled under .net 9 at least:
#if NET9_0_OR_GREATER const char ESC = '\e'; #else const char ESC = '\x1b'; #endif const string ESEQ = "${ESC}[";
Why just not using the EscSeqUtils class?
public const char KeyEsc = (char)KeyCode.Esc;
public const string CSI = "\u001B[";
Yes, that's the exact idea. I just forgot we had that, in that moment. 😅
In any case, the conditional to switch it out for \e is still relevant, though completely unimportant.
[everything about doing it via streams]
One additional potentially major upshot of this is it MAY offer a path to lift our curse(s), at least partially. 🤔
And note that the utf-8 string in that sample is not a string.
The u8 suffix makes it a ReadOnlySpan<byte> ONLY containing UTF-8 bytes as a compile-time constant. So that sample was pure UTF8 from code to console. No re-encoding, no allocations, no marshalling. Just raw bytes, only on the stack, and therefore like lightning compared to a "normal" string.
One unfortunate shortcoming of the current implementation of that is that string interpolation and u8 are mutually exclusive, even though interpolated strings can themselves be constants. 😞
oooo but I just had an idea...
Might be a way to achieve something similar. Gonna try it out later.
This is just a reminder to myself, for when I get back, so I don't forget the idea:
u8 + spread operator + collection initializers?
@tig, a question for you:
How many buffers would you like things to be using? Are we wanting to be double-buffering? Triple? More? We can probably afford to go pretty deep if we want, since we're dealing with fairly small screen space, so long as we refine some things like the PInvokes and the marshalling around them.
As I get into the weeds of this, I'm realizing there are some simple yet important things that can be done in modern .net and c# to drastically reduce the hammering on main memory and cache flushes that come along implicitly with how some of the old stuff works, due to memory barriers and other goodn't things that happen quite bit but which can now be mitigated, and in what end up being remarkably simple ways.
@tig, a question for you:
How many buffers would you like things to be using? Are we wanting to be double-buffering? Triple? More? We can probably afford to go pretty deep if we want, since we're dealing with fairly small screen space, so long as we refine some things like the PInvokes and the marshalling around them.
As I get into the weeds of this, I'm realizing there are some simple yet important things that can be done in modern .net and c# to drastically reduce the hammering on main memory and cache flushes that come along implicitly with how some of the old stuff works, due to memory barriers and other goodn't things that happen quite bit but which can now be mitigated, and in what end up being remarkably simple ways.
Previous to your work here, and my work on ConsoleDrivers last year, it was a true mess. What I focused on was correctness and simplicity of the API enabling reduction of duplicated spaghetti code in the the drivers.
I am not YET concerned about performance.
Until we get wide-char rendering and truecolor working on all drivers, AND we have a driver that works purely with ANSI escape sequences the concept of how many screen buffers we have is still in flux.
Thus, my recommendation is to hold off unless you have absolute clarity of how the above will be addressed within the scope of your design.
IOW: I am biased towards architectural/API cleanliness, and you are biased towards performance. We both tend to each a bit too hypothetically. I believe the tie-breaker should NOT be hypothetical performance issues. ;-)
I talk about performance a lot because that's just still pretty low-hanging fruit for us, here.
But the main goals I have nearly always originate from architectural improvements and quality of life improvements for both the consumer and their end users. Performance just goes hand-in-hand with it a high percentage of the time when one can write the same operation a little differently during that work and avoid at least a chunk of what made that particular unit expensive. And around PInvoke, those pitfalls are so numerous (in .net applications in general, just because of how all of that stuff works) that it starts to look more like a crater rather than individual pitfalls, and you just happen to have a giant dirt pile 3/4 the size of the crater right next to it ready to push in with a bulldozer.
As it turns out, a lot of what I'm doing in this work enables a pretty large amount of code re-use between the different drivers, which I know you'll love, and which was another of my main goals when sizing up what I wanted to work on for this change series. Performance just happens to be closely related to the things getting cleaned up, which makes me even happier and more motivated. Plus it's nice to get a tangible result. I've always liked improving products, tools, processes, etc every bit as much as creating new ones. Sometimes even more, since I actually have fun trying to get inside someone's head to see if our combined efforts can result in something even better in the end. 😁
Another of my goals for this that I'm pretty sure will make your day once it's ready is to completely eliminate more than one pretty large internal class, due to the cleaner architecture, by eliminating a ton of duplication and then merging what's left since even in their current states they already overlap quite a bit (for various reasons - organic growth can get that way sometimes).
So, if the performance wins we will get from the work I'm doing anyway don't pique your interest, then I'll slightly re-phrase the question:
The architecture in the console drivers is much more complex than it needs to be and can be greatly simplified by doing a few relatively simple things.[^butStill] One of those things requires me to be briefed on some intentions, or else it'll just be me doing that work unilaterally and then going "SURPRISE!" I would rather get your input, in case you had any insights, plans, prior experiences to watch out for, or even just preferences before I pick a design for that.
Just to save a back-and-forth:
I think double-buffering is plenty, but triple-buffering doesn't really take much additional work and could actually make it feel snappier to the end user, even though there's implicit latency added, because the next two frames can get most of their drawing work done before the current frame has finished getting shoved to stdout, which means much higher chance of having a fully-rendered frame ready on next write, ultimately reducing the incidence of tearing (exactly why it's used in 3D applications, too, especially when V-sync to the monitor is in use, which we kinda-sorta-almost-but-not-really have implicitly, in text mode).
As a side-note[^notEven] slightly more than tangential to the topic (secantial? reentrant? 🤔):
I tend to play the long game, strategically. Even though some of the things I'm doing right now will have very visible results to anyone working on the code of TG itself, some of it lays groundwork to make future work easier. And that's actually one of the biggest reasons behind my tendency to check the rulebook (harp on certain conventions): Because most of the ones I try to stick to have pretty solid backing behind them. But I don't think anyone's ever really pushed back on anything that I've made at least a half-decent case for, here, on technical grounds. But I also think my verbosity for sake of covering as many bases at once that I can comes off as though I feel much more strongly than I really do about a lot of things, because I've just found throughout life that things move along a lot quicker if I don't try to explain the 5 mile long train I'm on and just talk about how this particular car can be pimped out for now and then get to the rest later. 😅
Although my #1 preference is a small and passionate group of people with compatible personalities but at least different approaches/strategies for tackling whatever they're presented with, whiteboards all around the room, a whole package of different colored markers with everyone always having a marker in-hand, and everyone giving their thoughts openly and without fear of judgment, resulting in coming up with something awesome (even if awesome in a given context means remarkably simple/elegant) that everyone already groks from the start because it's got a little bit of each of us in every facet. 🙂
I had the perfect team for that, at one job, and loved it so much before [insert predictable story about ham-handed corporate organizational restructuring by new execs wanting to make a mark here] happened. Haven't quite had the same level of it since then, but there are moments. And that's where it becomes a secant rather than a tangent: It's why, even though, sure, I could just do the work and then modify based on any feedback anyone feels strongly enough to give, I instead prefer to ask for input on something fundamental like this one that I already have a half-formed opinion on but want the other perspective on, if there is one - even if it's just spitballing. I like spitballing. Got a straw right here in fact.
[^butStill]: And we'll still get the performance boost "for free," as a result of what the changes fundamentally represent. [^notEven]: More like side-book?
Oh whoops. Looking for a silly GIF made me forget the last thing I wanted to mention that was also a motivation behind specifically asking you the question, directly:
Regardless of the frequency or force with which someone puts their foot down in a project, I'm always cognizant about the artistic/creative element of what we as software engineers do, so there's an element of deference in the question, insofar as the shape of my work will morph to make something with at least a small metaphorical signature of the project principal. Heck, that could even just be liking "DuckBill" over "SchoolBus" for the name of a bright yellow-orange color or something. 🤷♂️
I love all of the 👆. I even read it all. Twice.
I'd like to see a double-buffer that could be extended to 3 design. The things I think / have thought about related to this:
TG needs to provide an abstraction layer above terminals.
- Standard implementations are getting better and more standardized, but we will ALWAYS have significant differences between Windows, Linux, Mac, WT, ConHost, iTerm, wizterm, footerm, ConEmu, blahTerm, XTerm, TermX, and GruntledTerminator.
- Fonts & Unicode & Color.
- Plus we have the dimension of the width and slipperyness (bandwidth and latency) of the pipe to the terminal. It is my intent that TUI developers should not have to think about those things unless they are doing something very bespoke.
- Oh, users have preferences. Some like mice. Some like clicky keys. Some like quiet keys. Some like it all. Some have 4 mouse wheels. Some have none.
Some proposed Tenets (in addition to those here: https://github.com/gui-cs/Terminal.Gui/blob/40056a6f2c410996537946a69e0f1b4a43a044db/CONTRIBUTING.md)
- Write Once, Run Anywhere.^IAmAHypocrite The abstraction layer that enables all of the above should be hidden, as much as possible, from TUI developers.
- Speed Matters Less Than Functionality. Breadth of cross platform support is more important than performance.
What others might you suggest?
Related issues where I've written thots:
- https://github.com/gui-cs/Terminal.Gui/issues/3622
- https://github.com/gui-cs/Terminal.Gui/issues/2319
- https://github.com/gui-cs/Terminal.Gui/issues/2747
- https://github.com/gui-cs/Terminal.Gui/issues/1316
- https://github.com/gui-cs/Terminal.Gui/issues/2933
- https://github.com/gui-cs/Terminal.Gui/issues/2610
- https://github.com/gui-cs/Terminal.Gui/issues/2803
- https://github.com/gui-cs/Terminal.Gui/issues/3119
- https://github.com/gui-cs/Terminal.Gui/issues/3413
- https://github.com/gui-cs/Terminal.Gui/issues/457
- https://github.com/gui-cs/Terminal.Gui/issues/3302
- https://github.com/gui-cs/Terminal.Gui/issues/666
Then there's the whole discussion of by whom and when Adornments should be drawn.
I'm PRETTY SURE I can make the current design work and fix this:
- https://github.com/gui-cs/Terminal.Gui/issues/2995
But, if I'm wrong, to get the amazing ability to have lines/text "auto join" even when Views are independent, we may need a compositor mechanism that's not LineCanvas.
I will know for sure when this is tackled:
- https://github.com/gui-cs/Terminal.Gui/issues/3407
Great stuff @dodexahedron .
Oh yea:
We need to recognize that by tackling
- https://github.com/gui-cs/Terminal.Gui/issues/2610
we are effectively building a new ncurses. It would be nice if ncurses wasn't such a POS, but it was designed and built too long ago and has too much weirdness/baggage as we've learned with CursesDriver.
Right now, the code in the UpdateScreen() implementations starts to lay the foundation for the non keyboard/mouse parts of that. But, as I think you've discovered, it barely works as-is.
Whoops. Apparently I didn't click the comment button hard enough this morning when I wrote this up. 😅
But I'll add a TL;DR, which is: 🥳
TL version follows:
Sounds like we were on the same page and didn't even realize it. 😆
I'd like to see a double-buffer that could be extended to 3 design.
Exactly what I was initially thinking. If it really isn't much more to get triple, when work is being done for that, I probably will just go ahead and go all the way from the start. Otherwise, yeah - I'd go for exactly that idea.
TG needs to provide an abstraction layer above terminals.
1000000%
And that's the most significant underlying goal of this change series, in fact, though not explicitly stated in case I wanted to break out into another change set instead. Probably will, anyway, just for organizational purposes, but the work being done right now is definitely aimed at violently shoving some things in that direction.
Fortunately for us, too, .net itself already does provide at least some abstractions, which are the most visible parts of what I'm adopting in this work. And the facilities for that stuff that are available in .net actively continue to improve in various ways - some subtle and some pretty important. So that's cool for us, but just a side-note here. But meshes with your next bullet, somewhat:
- Standard implementations are getting better and more standardized, but we will ALWAYS have significant differences between Windows, Linux, Mac, WT, ConHost, iTerm, wizterm, footerm, ConEmu, blahTerm, XTerm, TermX, and GruntledTerminator.
Absolutely. Even just look at the configuration screens in PuTTY for an abbreviated history and collection of various common quirks. 😅 Or at least I assume it still has that screen - ever since windows got openssh, I haven't used putty ever again.
And even within the same major.minor version of the same program, there can be differences that matter to us because of what TG is. Sucks for us, but that's kind of the point of a library like this. Yeah, we give you standard TUI controls, but we also let you do so the same way across at least a reasonable range of environments by doing that for you.
- Fonts & Unicode & Color.
It's almost funny to lump those three together in so terse a way in one little bullet, with how big each one is, even all on its own. 😅
- Plus we have the dimension of the width and slipperyness (bandwidth and latency) of the pipe to the terminal. It is my intent that TUI developers should not have to think about those things unless they are doing something very bespoke.
Yep. Although at least some of that we get to rely on the lower layers for (or should, to an extent, anyway). SSH, for example, is already over TCP, so order we receive things in isn't our responsibility (or place) to interpret in any other order than that in which it was delivered to us, since TCP won't be handing us anything not in the same order it was transmitted.
But yes - any session with a high BDP has high potential to deliver a suboptimal experience to the user. To that end, one of the most significant things we can do is one that's already in progress, which is adopting terminal control sequences wherever we can, to cut down on the impact of high BDP.
Later on, we could/should also take a pass over things to determine if there are existing operations that can be made lighter-weight by using more of those, as well.
But another broad thing about abstraction overall, applicable to all of the preceding points, is that the current state of the driver infrastructure/implementation is not extensible outside of TG itself. There are required pieces that are exposed in the public API surface, but impossible to implement due to those pieces having tight coupling with internal types.
That's something I'm fixing during this work, too and is a part of things I've been planning for a long time and have mentioned a few times this past year. That being interfaces.
Once I have gotten a little more done, I'm going to extract interfaces for everything in the driver stack and make it all use those interfaces. Then we can even make the actual implementations themselves internal if we really wanted to (probably shouldn't of course), because it will force de-coupling some things and, more importantly, will ensure that anything that is dependent on anything else in that stack is visible and has a guaranteed API contract.
I happen to have actually already done a bit of that on a separate branch, early on in this work, just to see how much work it would take. It's actually not that bad, now, thanks to other work that's been done along the way for V2 and what's being done related to this change series. The reason I paused that work was actually because that is what showed me a lot of what else needed to be done anyway, and I would have ended up deleting a bunch of work that the rest of this work will obviate. For the most part, though, outside of places where the coupling needs to be fixed, consumption of the interfaces when I was doing that were almost universally as simple as changing ConsoleDriver to IConsoleDriver in consuming code. 🥳
- Oh, users have preferences. Some like mice. Some like clicky keys. Some like quiet keys. Some like it all. Some have 4 mouse wheels. Some have none.
Ha yeah. Always fun. Another good place to ensure that we not only provide a solid base functionality, but also make extensibility possible without requiring recompiling TG itself.
Some proposed Tenets (in addition to those here: https://github.com/gui-cs/Terminal.Gui/blob/40056a6f2c410996537946a69e0f1b4a43a044db/CONTRIBUTING.md)
- Write Once, Run Anywhere.1 The abstraction layer that enables all of the above should be hidden, as much as possible, from TUI developers.
- Speed Matters Less Than Functionality. Breadth of cross platform support is more important than performance.
What others might you suggest?
Those are a solid starting point.
Maybe adding something that meshes with the last point above those, about respecting the user and not forcing things upon them.
Another that I've had to mention a couple times that I think deserves its own bullet: "Terminal.Gui is not the application." It is a library. It is just one frankly insignificant component consumed by an application, and needs to get out of the way for any and all purposes not directly related to user interaction. Thus, any assumptions made absolutely critically must be passed through that filter, or else we are putting the consumer in a frustrating box they didn't need to be put in. So, thread/concurrency-safety, lifecycles of anything that isn't strictly gen 0, resource consumption (of all types), accessibility, type safety, exception throwing and handling, and even simple matters of formality (at least in the public API surface) all should be given their due respect. For the most part, they usually are, or are at least thought about, but I think it is worth reminding any contributor that TG is not the application and needs to act that way.
Related issues where I've written thots:
Agreed at a high/conceptual level. Which actual type they shouldn't directly be using may not yet be fully fixed (or may just end up being a naming thing). But conceptually, right: No, they should not be directly using any of the low-level internal implementations in TG - only a public interface that lays out the required minimal contract to use them. Not allowing any derivation via a mechanism like that would be needlessly draconian, though, for no benefit to us. But I don't think that's your intent anyway.
Heh yep. This one I guess finally tackles that one.
I think the desired result can be achieved by simply utilizing the standardized logging framework abstractions provided by Microsoft.Extensions.Logging, which are pretty much ubiquitous elsewhere and are completely agnostic of framework/implementation.
Just add statements to log the low-level stuff at the appropriate level to an appropriate target and then, if/when needed, enable that in your config and it'll "just work."
Not to engage in post-mortem pugilism with this equine critter too much, but we should do that, regardless.
Yep. Ideally this change series will finally make that reasonably possible, or at least make it not impossible, so it can be addressed afterward.
I've been eyeing Cell a lot while working on this. I've got a bunch of different thoughts stewing on that topic. Will definitely have to come back to that once this is done.
Yep. The main pain point I think we'll have to deal with is that the level of support for VTS varies a fair bit from version to version and from host to host, on the same system. We just need to be sure we account for that in some way, even if it's as simple as sticking to the lowest common denominator for sequences we use, for the easy way out.
I'm pretty tired after the rabbit holes I went down (unrelated to TG) before coming back to this, so I won't give direct commentary on the rest of these bullets, since I have not yet looked them over, but the bullets themselves look like 👍 to me across the board:
- Harden
EscSeqUtils#2803- Figure out
VK_PACKET/ConsoleKey.Packetand build support for it #3119- Improve redraw performance by making
Clipbe aRegioninstead ofRectangle#3413- Colors: Update Color System (TrueColor, Blink, Underline, etc...) for v2 (Master Issue) #457
- View drawing APIs should restrict drawing to
Viewport#3302- Refactor ConsoleDrivers to remove duplicate code #666
Then there's the whole discussion of by whom and when Adornments should be drawn.
I'm PRETTY SURE I can make the current design work and fix this:
But, if I'm wrong, to get the amazing ability to have lines/text "auto join" even when Views are independent, we may need a compositor mechanism that's not
LineCanvas.I will know for sure when this is tackled:
And this work may also impact that. If all goes according to plan, life should be easier. 🙂
Great stuff @dodexahedron .
Footnotes
- https://blog.kindel.com/2013/02/21/james-gosling-screwed-us-write-once-is-anti-customer/ ↩
Addition not in the original:
Ha. I read the blog post today.
The last line of the last commenter's comment reads "Basically, doing it right ends up rewarding you in unexpected ways, and doing it wrong ends up biting you on the ass in unexpected ways."
I say scratch the "un" from the second to last word. 😆
I think I'm done with this one for now. I went back to working on continuing work on the next branch, so this one is ready as far as I'm concerned.
Pretty big improvements and consolidation already happening with WindowsDriver/WindowsConsole, so far. 😃
Oh and a minor informational side note:
I also loaded the code of this PR up in Rider on Windows and Ubuntu 24.04.1 to make sure things are still friendly to it, and to make sure tests also work in it as expected.
Went well. I didn't encounter any trouble, so looks like we're still good in Rider.
And I gotta say.... While I still don't care for Rider's overall environment, as it still feels perplexingly less capable than ReSharper somehow, it does work really well in the testing and debugging of testing.
The test runner, its output, the UI around testing, and especially coverage, profiling, and debugging of unit tests in this solution in particular do seem to work more reliably in Rider than it does in VS, on those two machines. 🤷♂️
(@tig you in particular may find this interesting... Or want the following few minutes back... One of those, probably...)
You know...
As I was working on the beginnings of a platform abstraction layer, today, I thought, "OK... There's no way this hasn't at least been partially done already, or else how can System.Console work at all, across platforms?"
So I went diving through the dotnet source on github, at the v8.0.8 tag.
There's a wealth of stuff in there that is exactly what we need/want for quite a few scenarios (or pretty close), some of which I was beginning to write almost verbatim re-implementations of, but won't have to now that I've found them. ...But they're all internal types, in .net 8, with a small number marked public in .net 9. But hey - doesn't mean I can't use it now (yay open source with same license)![^standingOnShouldersOfGiants] There's also plenty in there that, while not necessarily directly useful, provides excellent templates for or at least inspiration for what I do need.
I don't need the full implementations of a lot of it - just some types, marshallers, and a base class or two to cut the code I have to write for the custom marshalling down by... a lot...[^suchShort]
While I'm mostly talking about marshalling and PInvoke, specifically, here, it's not limited to that. But that's a big piece of the puzzle, and I'm eager to get back to work on it, so I'm holding myself to a 20-page limit on this comment.[^veryBrevity] 😛
If I do end up taking any actual code directly from the runtime, I'll keep them API-compatible just in case we are granted any gifts of more of them becoming public in the future.
But...
Shifting gears back to the topic of abstractions...
Platform interop, especially if properly abstracted, is an independent, separable component of the library/framework, and there are tangible benefits to separating it to its own component library, as well as potential negative impacts if that isn't done.
Why?
For source-generated PInvoke ([LibraryImport]) to work fully properly, consistently, and deterministically, and to keep it trim-safe, one must use the [assembly: DisableRuntimeMarshalling] attribute, or else the DllImports that LibraryImport generates can still end up needing runtime code generation, which is not trim-safe. And notice the assembly in that. It's an assembly-wide attribute, and that is the only level it is allowed at (and that makes sense, considering what it is). It's even a code fix built in to Roslyn to insert it for you, since it's kind of a bad idea not to be using it, with source-generated interop (and sometimes even with old-school interop, but mostly (not exclusively) in trimming situations).
Why is it being an assembly-level attribute a problem, though (thus prompting the need for separation)?
Because not only does it affect DllImport and LibraryImport, it has effects on more than just them, as well.
A biggie for those two (so...PInvoke), specifically, is that in, out, and ref parameters are not handled for you anymore, nor are any types that are not already blittable, and thus those things can't be used on the native method (the extern that gets the DllImport, whether you wrote LibraryImport or DllImport).
All non-source-generated marshalling is off, so you have two options:
- Use only fully blittable types in c#-land, without being able to use things like
[MarshalAs]and make PInvoke signatures only use those types, never as references (so can't use half of the Console methods), the short list of built-in types that are blittable or have marshallers defined, or use pointers directly, which gets ugly quite quickly and should be beneath the abstraction layer, since this is C# and we shouldn't be exposing unmanaged stuff raw like that anyway. - Write the glue marshaling code that the Roslyn generator[^ITerkErJerb] uses during source generation of LibraryImport PInvokes. I'm essentially doing a non-generic version of that work, in what I have pushed so far to the work branches for this so far, so it's actually not really any additional work, plus provides a promise of consistency since type marshalling is handled by a very straightforward system specifying managed type, direction and intent of marshalling, and which marshaller does that stuff.[^manThisGotTooLongForANumberedList]
Aside from the PInvoke impact, it also alters how some delegates are handled internally, and imposes strict limitations on the types used for the extern methods, because they MUST comply with the rules/list here. The flip-side of that is that those restrictions make the rules for what can be used quite simple. As above, things either need to be fully blittable and identical to their native representation, OR there must be a marshaller defined that the source generator can make use of.
In any case, I'm pretty sure it would break some of the existing pre-cached delegate implementation inherited from V1 (which is already on the chopping block anyway, so no loss there), but I'm really not interested in trying to find out what, where, and why things are broken by that, since it can all be avoided by just putting interop into its own assembly and doing things the way the native interop docs describe. Then, interop code can be under the auspices of the [assembly: DisableRuntimeMarshalling] attribute, while the rest of the TG code gets to be blissfully unaware of and free of its edicts.
Another really nice benefit to that is that, if I write it as a base abstraction layer[^interYoFACE], with the platform-specific implementations inheriting from/implementing that API, it becomes naturally pluggable, and consumers can leave out the platform-specific assemblies they don't want or need, if size or attack surface or anything else related to dead code is a concern.
I wrote up a bunch of POCs today in a toy app through which I pass darn near everything I write of this nature[^soLonely], to feel out all the concepts involved in all this and more, including custom marshallers (which will mostly or possibly completely eliminate my explicit hand-written PInvoke code layer), safer memory management, buffer pooling, and various console and platform abstraction concepts to make our and consumers' lives easier. Lots of good results, lots of good learning, and some interesting exposition of issues that need to be addressed even if this whole exercise were to be scrapped and the existing implementation used for the time being - some of which are security-critical to a rather frightening degree[^skyIsFalling], but easy to fix and getting fixed, no matter how the rest of this work evolves.
Alright, I wanna get back to work. This is plenty of stuff to chew on, anyway.
...Hey! I kept it under the character limit![^wow]
[^standingOnShouldersOfGiants]: And it's perfectly trustworthy stuff, too, because it's what the runtime would use for its compile-time code generation by Roslyn (who is delightful) or run-time IL generation by Ryu (which we can't rely on for all deployment types), as applicable to each piece, for native interop.
[^soLonely]: Guarantees isolation from the rest of the library, since it's JUST that code, and I can hammer the hell out of it before using it in the library.
[^ITerkErJerb]: Whose job is effectively what I'm already doing by hand, anyway, so she can have that back.
[^dontWorry]: Not to worry. That doesn't mean consuming code suddenly turns ugly. In fact, the entire point of all this is to make the interop code do all that junk for you, so that you, at the call sites for those methods, no longer even have to do things like providing access flags or using some cumbersome semi-native-equivalent struct. Instead, that work gets done in design/compile-time generated source, which you can even inspect if you want, and you can just use things in a much more natural idiomatic C#/.net way, rather than feeling Win32/ncurses/libc/etc's influence directly. You know... abstraction!
[^suchShort]: Wait... The machine can do some work for the human? SIGN ME UP!
[^veryBrevity]: Well... maybe 21... We'll see...
[^wow]: Clearly, Doge wrote some of the footnote tags.
[^skyIsFalling]: Depending on specific spot in the TG code, we're talking 8-10, in CVSS terms. Mostly 10s. Privilege escalation, arbitrary code execution, and more, and they don't need root to exploit. Though one I wrote a PoC of did get the assembly file deleted immediately by MS Defender upon copying it out of the dev drive, due to what it was doing being pretty clearly suspicious (trying to snoop stuff out of LSASS - a historically popular target or component of a wide range of game over-level attacks).
[^interYoFACE]: I'm going to do that either way, using interfaces and such
[^manThisGotTooLongForANumberedList]: It's actually already explicitly in use in code you can see on the first branch of this, with uses of types like SafeHandleMarshaller<SafeFileHandle>.ManagedToUnmanagedIn and family, which are the built-in "custom" marshaller types for SafeFileHandles, which you've been using in every console application you've written in .net 7+ without even realizing it (again - yay abstraction layers), and something similar before that, but not as pretty. That code deals with all the pointers and allocations and pinning and stuff that you really don't want to have to think about.[^dontWorry]
Very interesting.
🤨
I've not looked at the .Console code. Glad you have.
I'd be curious to know what's going on relative to this:
https://github.com/dotnet/runtime/issues/96490