search

gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.74k stars 695 forks source link

Refactoring, cleanup, and abstraction for ConsoleDrivers #3692

Open dodexahedron opened 3 months ago

dodexahedron commented 3 months ago

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:

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.

dodexahedron commented 3 months ago

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.

dodexahedron commented 3 months ago

@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.

dodexahedron commented 3 months ago

@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:

image

dodexahedron commented 3 months ago

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.

image

dodexahedron commented 3 months ago

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.

tig commented 3 months ago

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.

dodexahedron commented 3 months ago

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.

dodexahedron commented 3 months ago

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.

tig commented 3 months ago

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!

dodexahedron commented 3 months ago

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.

dodexahedron commented 3 months ago

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.

dodexahedron commented 3 months ago

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. 🤷‍♂️

dodexahedron commented 3 months ago

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:

image

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.

dodexahedron commented 3 months ago

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}[";
BDisp commented 3 months ago

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}[";

Why just not using the EscSeqUtils class?

public const char KeyEsc = (char)KeyCode.Esc;
public const string CSI = "\u001B[";
dodexahedron commented 3 months ago

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.

dodexahedron commented 3 months ago

[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. 🤔

dodexahedron commented 3 months ago

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. 😞

dodexahedron commented 3 months ago

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?

dodexahedron commented 2 months ago

@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 commented 2 months ago

@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. ;-)

dodexahedron commented 2 months ago

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.

SpitBallSleepingGIF

[^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?

dodexahedron commented 2 months ago

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. 🤷‍♂️

tig commented 2 months ago

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.

Some proposed Tenets (in addition to those here: https://github.com/gui-cs/Terminal.Gui/blob/40056a6f2c410996537946a69e0f1b4a43a044db/CONTRIBUTING.md)

What others might you suggest?

Related issues where I've written thots:

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:

Great stuff @dodexahedron .

tig commented 2 months ago

Oh yea:

We need to recognize that by tackling

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.

dodexahedron commented 2 months ago

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:

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

  1. 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. 😆

dodexahedron commented 2 months ago

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. 🤷‍♂️

dodexahedron commented 2 months ago

Whoops. I meant for that to go in the PR.

Gonna take care of that...

dodexahedron commented 2 months ago

(@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:

  1. 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.
  2. 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]

tig commented 2 months ago

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

dodexahedron commented 2 months ago

I've not looked at the .Console code. Glad you have.

It's all over the place throughout coreclr, with pieces in at least 6 different libraries . In fact, a fair amount of the platform interop code is actually separated into specific DLLs for various reasons including that those have DisableRuntimeMarshalling specified, but most of the rest of coreclr does not. A lot of that used to be in C, C++, or C++/CLR, but they've really been making an effort to move as much of everything everywhere into pure C# as they can in the past several years, since pinning is expensive, among other reasons.

I'd be curious to know what's going on relative to this:

Yeah there are definitely some fun bits.

It's kinda interesting/odd to me that all the PInvoke code, including types, functions, constants, and marshalling, where necessary, sometimes even with XmlDoc comments, is already in .net in classes named for the library they're in, such as Interop.Kernel32, which, predictably, has a ton of stuff from kernel32, including the specific console functions used by coreclr on Windows. Yet, they're all internal and things like CsWin32 still exist. 🤷‍♂️

As for Console, the underlying platform code for parts of it (which is one of many places coreclr has a PAL) of course uses quite different APIs on each platform, particularly around HCI like mouse input, but then the same (or at least similar) APIs for other bits where possible.

Ironically, on Unixy platforms, one of the methods of a class called StdInReader, which is simply an internal sealed class derived from TextReader (which is what Console.In is), has this to say on its implementation of ReadKey:

  /// Unlike Windows, Unix has no concept of virtual key codes.
  /// Hence, in case we do not recognize a key, we can't really
  /// get the ConsoleKey key code associated with it.
  /// As a result, we try to recognize the key, and if that does
  /// not work, we simply return the char associated with that
  /// key with ConsoleKey set to default value.

And here's the code of StdInReader.ReadKey as of the v8.0.8 tag:

        public ConsoleKeyInfo ReadKey(out bool previouslyProcessed)
        {
            if (_availableKeys.Count > 0)
            {
                previouslyProcessed = true;
                return _availableKeys.Pop();
            }

            previouslyProcessed = false;
            return ReadKey();
        }

        private unsafe ConsoleKeyInfo ReadKey()
        {
            Debug.Assert(_availableKeys.Count == 0);

            Interop.Sys.InitializeConsoleBeforeRead();
            try
            {
                if (IsUnprocessedBufferEmpty())
                {
                    // Read in bytes
                    byte* bufPtr = stackalloc byte[BytesToBeRead];
                    int result = ReadStdin(bufPtr, BytesToBeRead);
                    if (result > 0)
                    {
                        // Append them
                        AppendExtraBuffer(new ReadOnlySpan<byte>(bufPtr, result));
                    }
                    else
                    {
                        // Could be empty if EOL entered on its own.  Pick one of the EOL characters we have,
                        // or just use 0 if none are available.
                        return new ConsoleKeyInfo((char)
                            (ConsolePal.s_veolCharacter != ConsolePal.s_posixDisableValue ? ConsolePal.s_veolCharacter :
                             ConsolePal.s_veol2Character != ConsolePal.s_posixDisableValue ? ConsolePal.s_veol2Character :
                             ConsolePal.s_veofCharacter != ConsolePal.s_posixDisableValue ? ConsolePal.s_veofCharacter :
                             0),
                            default(ConsoleKey), false, false, false);
                    }
                }

                return KeyParser.Parse(_unprocessedBufferToBeRead, ConsolePal.TerminalFormatStringsInstance, ConsolePal.s_posixDisableValue, ConsolePal.s_veraseCharacter, ref _startIndex, _endIndex);
            }
            finally
            {
                Interop.Sys.UninitializeConsoleAfterRead();
            }
        }

In the ConsolePal.Windows.cs version, ReadKey is handled at that layer and doesn't wrap the stream in a bespoke class, because it just immediately runs to the Win32 PInvokes, rather than using streams, since Windows doesn't include anything but keys when you read the handle as a normal file/stream, requiring you to use the ReadConsoleInput function to get the other stuff.:

public static ConsoleKeyInfo ReadKey(bool intercept)
        {
            Interop.INPUT_RECORD ir;
            bool r;

            lock (s_readKeySyncObject)
            {
                if (_cachedInputRecord.EventType == Interop.KEY_EVENT)
                {
                    // We had a previous keystroke with repeated characters.
                    ir = _cachedInputRecord;
                    if (_cachedInputRecord.keyEvent.wRepeatCount == 0)
                        _cachedInputRecord.EventType = 0;
                    else
                    {
                        _cachedInputRecord.keyEvent.wRepeatCount--;
                    }
                    // We will return one key from this method, so we decrement the
                    // repeatCount here, leaving the cachedInputRecord in the "queue".

                }
                else
                { // We did NOT have a previous keystroke with repeated characters:

                    while (true)
                    {
                        r = Interop.Kernel32.ReadConsoleInput(InputHandle, out ir, 1, out int numEventsRead);
                        if (!r)
                        {
                            // This will fail when stdin is redirected from a file or pipe.
                            // We could theoretically call Console.Read here, but I
                            // think we might do some things incorrectly then.
                            throw new InvalidOperationException(SR.InvalidOperation_ConsoleReadKeyOnFile);
                        }

                        if (numEventsRead == 0)
                        {
                            // This can happen when there are multiple console-attached
                            // processes waiting for input, and another one is terminated
                            // while we are waiting for input.
                            //
                            // (This is "almost certainly" a bug, but behavior has been
                            // this way for a long time, so we should handle it:
                            // https://github.com/microsoft/terminal/issues/15859)
                            //
                            // (It's a rare case to have multiple console-attached
                            // processes waiting for input, but it can happen sometimes,
                            // such as when ctrl+c'ing a build process that is spawning
                            // tons of child processes--sometimes, due to the order in
                            // which processes exit, a managed shell process (like pwsh)
                            // might get back to the prompt and start trying to read input
                            // while there are still child processes getting cleaned up.)
                            //
                            // In this case, we just need to retry the read.
                            continue;
                        }

                        if (!IsReadKeyEvent(ref ir))
                        {
                            continue;
                        }

                        if (ir.keyEvent.wRepeatCount > 1)
                        {
                            ir.keyEvent.wRepeatCount--;
                            _cachedInputRecord = ir;
                        }
                        break;
                    }
                }  // we did NOT have a previous keystroke with repeated characters.
            }

            ControlKeyState state = (ControlKeyState)ir.keyEvent.dwControlKeyState;
            bool shift = (state & ControlKeyState.ShiftPressed) != 0;
            bool alt = (state & (ControlKeyState.LeftAltPressed | ControlKeyState.RightAltPressed)) != 0;
            bool control = (state & (ControlKeyState.LeftCtrlPressed | ControlKeyState.RightCtrlPressed)) != 0;

            ConsoleKeyInfo info = new ConsoleKeyInfo(ir.keyEvent.uChar, (ConsoleKey)ir.keyEvent.wVirtualKeyCode, shift, alt, control);

            if (!intercept)
                Console.Write(ir.keyEvent.uChar);
            return info;
        }

For the record, one of the comments in that code that says they might be able to use Console.Read but it might break certain things is something I happened to confirm for myself, by chance, when I was trying to do exactly that in a PoC yesterday. You end up having to call additional kernel32 functions first to make it work like one would expect if you do that, and it has high potential to be fragile if a lot of care is not taken around various contingencies. So, for windows, it's best to just KISS and use ReadConsoleInput to drain the input buffer, as that code does, when all HCI input is wanted. And I have a suspicion that this Windows Terminal issue that another comment in the code mentions (microsoft/terminal#15859) may have something to do with the odd behavior around things you raised your issue over in the dotnet/runtime repo for.

At the end of the day, the fundamentally different way that Unix and Windows treat the IO streams for TTYs[^SiouxDough] creates the precondition for the different results, and historical baggage plus trying to help perhaps a bit too much in Windows. kernel32 involves itself in the interpretations of the bytes, rather than just being a dumb pipe,[^TsuDo] making VTS the platform's responsibility, rather than the terminal emulator's responsibility, which makes things more complicated, at the lower level. Or at least that part stands out to me as a kink in the process, anyway. Ironically, unix not special-casing it ends up making life easier since it's your terminal emulator's responsibility to interpret the bytes you give to its stdout stream, and the OS just hands them to it and gets out of the way for the most part. Buuuut then you want mouse input too, and...well... fun!

As I've been deep-diving into it all, I've more than once found myself wondering if allocating in-memory streams/buffers over a screen buffer equivalent[^sudo], allocating our own hosted PTY[^SueDoe], and just proxying the IO to the console the application was handed at launch might be the fix needed to make it all work consistently and more simply. But I have more research and prototyping to do to see if that's viable or even actually solves what I'm hypothesizing it would solve. But it sure would be a nice help in making a unified platform-agnostic abstraction if we could deal with all platforms using a PTY. That also makes network scenarios easier, as well, since a PTY has no requirement to be attached to the application hosting either of its streams. That, incidentally, also creates cool possibilities of things like a network driver that gives you a direct remote console to a running application on another machine, like a text-mode X sort of situation, not needing ConHost.exe or bash or whatever terminal emulator to even be in the picture.[^sudoku]

Another somewhat useful resource, but only for insight (due to incompatible LGPL-2.1 licensing) has been ReactOS, since that attempts to be a clean-room API- and ABI-compatible implementation of the Win32 APIs, from user space down to kernel space, including driver compatibility. Just sucks that LGPL-2.1 is viral and requires the entirety of a combined work using it to then also be under that license.[^boo] 😤 <--(I had a lot of trouble trying to get github to let me use that emoji)

Anyway, back to platform abstractions...

It's awesome that Windows Terminal is what it is, because work on that is forcing some attention to be paid to System.Console and related code/functionality that we're getting direct and indirect benefits from, through the changes/enhancements being made in coreclr to support it, with apparent respect and care being given to cross-platform implications of it all.

I've been watching progress at the WT repo and using it as a reference resource, as well, since obviously there's a teensy bit of relevance, there, for what we're doing.

[^SiouxDough]: Unix being simpler largely thanks to "everything is a file". [^sudo]: e.g. using an ArrayBufferWriter<byte> [^SueDoe]: On windows that'd be CreatePseudoConsole and on Unix it'd just be opening file streams over /dev/pty [^TsuDo]: Well I mean you CAN use the raw file stream exclusively, if you don't care about the mouse for actual input purposes, anyway. [^sudoku]: Though those of course would still be the majority of use cases anyway [^boo]: Can I just say that, philosophically, I respect but strongly dislike GPL for strong-arming itself into other people's work, beyond a narrowly-defined scope of JUST the code licensed under it? BSD, MIT, and MPL for the win. That's actual freedom. Live and let live. 🥰

dodexahedron commented 2 months ago

Another thought about something that just kinda amuses me in a way is that the code quoted above does some work that we then undo and then redo in various places. In particular, that's around modifier keys, which you can see the handling of at the bottom of the second code block (the windows version of ReadKey).

Not sure what, if any, relevance that will have as work continues, but it did stand out to me. If anything, maybe it'll help inform how the abstractions take shape. 🤷‍♂️

dodexahedron commented 2 months ago

Quick update:

Have been continuing to research and prove things out in simple isolated toy apps for various concepts.

When I re-branch for this, I'll have some nice new tools in the toolbelt for simplifying and improving the drivers and abstractions.

But today and probably tomorrow as well, I have some other pressing work that I need to get done not related to TG.

dodexahedron commented 1 month ago

OMG. I can log into github again! I've been back for like a week after the stuff i mentioned was finally over/completed, but was having trouble here...

Sorry. Mostly my fault, as it turns out. But it's fixed in a 3x failsafe way now. 😅

Anyway...

I'm back and will be getting myself back up to speed over the next few days, as far as work that's happened in the last few weeks, so I can get back to work on this.

Haven't looked over anything in my gh inbox yet, though. Just had this up toward the top and wanted to make contact so y'all know I'm alive and didn't ditch ya or something. 😅

tig commented 1 month ago

Missed you!

tznind commented 1 month ago

@dodexahedron I have started simplifying NetDriver as part of my work adding AnsiResponseParser (AnsiResponseParser changes are #3791 , net driver changes are https://github.com/tznind/gui.cs/pull/172/files)

Hopefully that won't cause too much conflict with your work here.

I am basically

The diff (to the main ansi branch) can be seen here:

https://github.com/tznind/gui.cs/pull/172/files

In fact we can probably even use something like:

private readonly BlockingCollection<InputResult> _resultQueue = new (new ConcurrentQueue<InputResult> ());
dodexahedron commented 1 month ago

Since I backed it out to do something even cleaner in the first place than what I had previously submitted a PR for, there's no conflict, right at the moment.

But, I would suggest/recommend that you use FileStreams to the pty anyway.

A queue ends up not really being necessary, though you certainly can use that on top of things.

FileStream is a fully cross-platform means of dealing with the terminal and is how libc does it and ultimately how win32 does it, as well (but windows doesn't implicitly separate stderr and stdout unless you tell it to, so they are different handles to the same stream by default).

Doing that, you have direct access to the raw buffer, avoiding all the marshaling overhead and complexity involved with a lot of the pinvokes that are currently in place.

Some are still necessary, for certain bits, particularly on Windows, but they all get a common base implementation, which is so lovely. :)

I'll just start up again on not-NetDriver when I get back to it.

dodexahedron commented 1 month ago

It's kinda interesting.

It's a FileStream, but it feels and behaves almost exactly like a NetworkStream on top of a UDP Socket or other unordered pair of unidirectional channels. It's forward-only/non-seekable, for both input and output, and you'll receive things in the order the OS dispatched them to you, which is what you'll get any other way, but with less junk in between and fewer platform-specific idiosyncrasies to deal with.

It's one place where Windows really shows its original roots and inspiration, because the console is really two character device files that you write to and read from (called $CONIN and $CONOUT), just like Unix, where literally everything is a file. WinNT and VMS share interesting history there. There's a third virtual character device file that is r/w, called just CON, and that was a handy thing to know back in the DOS days if you didn't have a text editor available. You could copy CON somefile.txt and then anything you typed was written to that file (which could even be a printer, which is also a virtual character device file) until you gave it a ctrl-z to signal end of stream.

Fun stuff.

Oh yeah - and you can still do all of that - even the printer output. Just pipe or copy to PRN (for default printer) or to the name of the printer port[^tharBeDragons] and there ya go. Although PowerShell has Out-Printer, too, which is easier to grok, and there's print.exe, but that's text only.

[^tharBeDragons]: Which might be ugly if you let it autoconfigure a modern WSD printer and didn't rename the port.