-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Evaluate feature parity between Windows Terminal sequences and existing backends #251
Comments
Is it safe to assume that if we get a positive (or almost positive answer) to this issue, and we manage to migrate Relevant issue: MicrosoftDocs/Console-Docs#289 |
...and we have to compare everything by hand, is there a documented list of terminal features/capabilities that are required for |
A bit of digging on bracketed paste: it kinda works, according to microsoft/terminal#395; most notably microsoft/terminal#395 (comment) and microsoft/terminal#9034 |
What do you mean by "positive answer"?
Whether that migration is appropriate is undecided as far as I'm concerned. I doubt that supporting multiple platforms at that level of abstraction will be necessary, and the more I think about it, the more I'd like to move to a situation where the only code that needs to touch Fds is a separate
No, I don't think that's all that we need in order to settle the question. I suspect it's not that straightforward, and that there are many little things that may need to be investigated (such as the stuff you linked to!). |
Sorry, I could have written that phrase in a better way. What I wanted to say was "terminal features required by
I guess the approach with separate
Let's hope for the best and prepare for the worst. After all, |
Possibly; the internal abstractions are probably already pretty close (
Exactly my thought as well; in Unix we have a lot of sins to atone for due to the long history of terminal emulation, but hopefully when it comes to supporting Windows it will be easier to know what "support" needs to mean. |
Well, this is news to me. Have you been hacking on Vty? |
Indeed, and "hacking" was really close to the meaning of the original word. Here's a summary $ git status -uno
On branch master
Your branch is up to date with 'origin/master'.
Changes not staged for commit:
(use "git add/rm <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
deleted: cbits/get_tty_erase.c
deleted: cbits/gwinsz.c
deleted: cbits/gwinsz.h
deleted: cbits/set_term_timing.c
modified: demos/Demo.hs
modified: src/Graphics/Vty.hs
modified: src/Graphics/Vty/Attributes/Color.hs
modified: src/Graphics/Vty/Config.hs
modified: src/Graphics/Vty/Inline/Unsafe.hs
modified: src/Graphics/Vty/Input.hs
modified: src/Graphics/Vty/Input/Loop.hs
modified: src/Graphics/Vty/Input/Terminfo.hs
modified: src/Graphics/Vty/Output.hs
deleted: src/Graphics/Vty/Output/TerminfoBased.hs
deleted: src/Graphics/Vty/Output/XTermColor.hs
deleted: tests/programs/VerifyEvalTerminfoCaps.hs
deleted: tests/programs/VerifyOutput.hs
deleted: tests/programs/VerifyParseTerminfoCaps.hs
deleted: tests/programs/VerifyUsingMockInput.hs
modified: tests/vty-tests.cabal
modified: vty.cabal
Untracked files:
(use "git add <file>..." to include in what will be committed)
log.txt
src/Graphics/Vty/Output/TerminfoBased.hs.prev
src/Graphics/Vty/Output/WT.hs
src/Graphics/Vty/Output/XTermColor.hs.prev
stack.yaml
stack.yaml.lock
no changes added to commit (use "git add" and/or "git commit -a") There is quite a set of limitations, most of them being my limited knowledge of inner workings of I generally followed the plan outlined by @noughtmare in #219 (comment) Major limitation for now - user input does not work as it should, I need to press |
Status report (with obligatory disclaimer
On this note, my time budget for OSS is depleted for the foreseable future, so my workings will lay dormant. |
Thanks for your update! I wanted to comment on this specifically: it's way too early to be concerned about implementing a vty-windows package. This early-stage exploratory hacking is exactly what we need right now but the final result will probably involve a clean-slate implementation anyway. There is also at least one other person working on this sort of thing right now, and I suspect your work would be helpful for them. Could you commit what you have on a fork and provide access to it? |
Will do sometimes next week, most probably on Tuesday. |
Okay, thank you! |
Hello Jonathan and ShrykeWindgrace, I've been doing similar hacking and have similar results. Perhaps we can compare notes, and split up the remaining investigation?
So there is a fair bit more work to reach feature parity. I'm also limited as ShrykeWindgrace is in the amount of time I can dedicate to this. Its just a hobby for me, but one I really enjoy. As for how we might begin to split the library into core/windows/posix pieces, my fork shows the modules that will need to change to support both platforms. I replaced all the types/functions that we used from the terminfo/unix libraries with internally defined types/functions. So that might give us a starting point to begin with. I am curious to how we might go about this. Here are types that were used: And all the functions: Look forward to working with you guys... |
Sounds like it may have been caused by GHC issue #21665 which was fixed in 9.4.2. |
Yup - thank you for writing that up! That was probably the only obstacle remaining before we could really get vty to work in Windows. |
RE: mouse input. That's most probably because it's disabled and we need to explicitly enable it. In a ghci session I get
in other words, |
And another observation - I've been browsing through rust's |
As promised, my fork, use it at your own risk =) https://github.com/ShrykeWindgrace/vty Tested against ghc-9.4.4
What I did not implement and will not have time to do in the next couple of weeks:
I still want to hack I'll be available to answer questions on my code on github. Cheers! |
Hello @ShrykeWindgrace,
So back to the original point of all this, @jtdaugherty asked us to evaluate Windows support for the capabilities in TermInfoBased and XtermColor. I will have to go through and check each one, but it appears that all the capabilities that vty needs 'just work' in Windows. (this is mainly based on the fact that all the tests in interactive_terminal_test passed). |
@chhackett Thanks for the comprehensive review =)
That's a very good point! I doubt that my setup would allow for that - both code structure and my laptop configuration are against that (I do not have a proper WSL2 =( )
Good to know. Does your implementation handle mouse moves? Or only mouse buttons?
Do you get resize events? How do you handle them? I did not find a proper way to resize the screen buffer, MS documentation warns against that idea, and Win32 does not even expose necessary API.
Quite probably. Since my terminal/shell is set with, essentially, |
On a side note, I tried compiling
I am inclined to add the special treatment for the case "input available, but of length zero" and manually send |
And a couple of questions:
|
Oh, yeah, I had similar kinds of problems mostly with my work laptop because they lock down features. It sucks.
So it gets events for clicks and drags, but not all mouse moves. I think that's the expected behavior after reading the docs on mouse modes for xterms. (and that's how it behaved in my wsl2 window)
Nope, don't receive events, just poll for them. It would be wonderful if we could get VT sequences for window resizes... So none of this involves calling setWindowSize. And I haven't really tested that functionality I guess. I'm not even sure I understand the use case. Does an app call that to make the screen larger (than the visible window)? Does that enable scroll bars? What happens when you start scrolling around the screen? What if you set it smaller than the visible window? I have so many questions. And here's the code I came up with this morning (might make more sense if you just check out my code from github - I changed the type signature of the 'install...Handler' functions slightly): type WindowChangeStateM a = StateT (Int, Int) IO a -- This function throws away the thread id. The new thread can't be stopped. This seems to be ok though? runWindowChangeLoop :: IO () -> IO () windowChangeLoop :: IO () -> WindowChangeStateM () Btw, I'm not sure whether we need to do anything with the 'continue process' handler on windows. I guess I don't know if windows has a similar facility for process stop/resume like the posix SIGSTOP/SIGCONT signals. |
On in Windows Terminal this could be problematic: microsoft/terminal#4417 and microsoft/terminal#5094 off the top of my I"ll try to test the reception of window resize events (VT or anything else) - not earlier than next Monday, though. |
Good catch! I tried it and it totally freezes the console window. Nothing to do then but kill the window, lol. |
I think you can probably do this more efficiently by waiting for a |
I still do not know to get these events from stdin properly. As far as I understand the documentation, if I get mouse events (I get them), I should get resize events, too, but I don't. Maybe that's because resize events are not sent as VT sequences? We could in theory use ReadConsoleInput from console's API, but I'd really like to avoid that. On an unrelated note, I managed to get Focus events working - they require additional configuration for stdin, they are disabled by default. |
Oh, thanks for pointing that out. I obviously don't know the Win32 API. Anyway, I'll switch to using that method. Also, ReadConsoleInput looks like a good candidate to add to the Win32 library, so I'll open a request there and see if someone is kind enough to add it, or I can add it myself. |
Rather outdated, but possibly useful: https://terminalnuget.blob.core.windows.net/packages/TerminalSequences.html#window-management |
Two immediately useful pieces of info:
|
Thanks for posting that question. It does confirm what I've been learning for the past week. So I just finished implementing the code to read window resize events via ReadConsoleInput and discovered a depressing fact. Which is that when you consume events via ReadConsoleInput (which includes key and mouse events), those events are not sent to stdInput via VT sequences. This makes me really sad, because now we face a choice of designs:
Solution 1 Pros:
Solution 1 Cons:
Solution 2 Pros:
Solution 2 Cons:
I'll leave it to you guys to decide what the desired approach is. But I'll come up with an implementation using ReadConsoleInput so we can gauge the impact this would have on the library design.
I'll just comment that I'm not sure we would even care about the VT sequences anymore if we go with ReadConsoleInputW... |
@jtdaugherty @ShrykeWindgrace @noughtmare
|
@chhackett I got quite the opposite impression/approach to the solution 1'
Again, to my understanding, this fits in the For example I'll try to make the necessary bindings next week, but anyone already has them, I'll be grateful=) |
I am logging events that I receive via ReadConsoleInput (focus enabled, mouse buttons enabled, window enabled, mouse move disabled)
I we squeeze hard enough on the Basing on these logs, I am quite confident that |
Yet another argument against reading raw bytes from haskell's And the root cause:
|
My understanding is that Windows uses UTF-16 encoding for all IO. I'm not sure if the encoding is different for stdin vs readConsoleInput. We will have to re-encode character input. Anyway, I've been working on implementing readConsoleInput for the last week or two, and finally got it mostly working. There is still a problem though. My implementation reads one character at a time. As a result, escape sequences are received as separate key events, and vty parses each byte of the escape sequence as an input character. (You noticed this as well. With posix, each escape sequence is received in a single block of bytes, which the parser processes correctly. But received as a list of single bytes, it doesn't. If you know the fix @ShrykeWindgrace I'd be happy to hear it. |
I am not sure about In my tests I recover at most one haskell's I have no other ideas right now that would allows us (at the same time):
I'll commit my implementation by the end of next Tuesday (CET). Monday, if my free time allows for that=) |
@chhackett @jtdaugherty as promised, I pushed to my fork of |
@ShrykeWindgrace |
@chhackett I briefly examined that package a couple of weeks ago, it seemed abandoned to me (latest hackage upload in 2016, no issues, unmerged PR since 2016, etc). IIRC, it did not implement functions like |
@ShrykeWindgrace Anyway, our prototypes of vty are pretty feature complete now. I'm going to wait and see how @jtdaugherty would like to proceed with breaking things up. |
@chhackett @ShrykeWindgrace that's great news! I have been following this comment thread casually as I've waited to see what you all came up with, but I am not very informed about all of the discussion that has happened along the way. Could one of you help me out by summarizing the status of your respective efforts? Things like:
|
MissingWhat is missing in my fork:
Otherwise, my daily driver Gross/uglyVT sequences for terminal capabilities are parsed (I copied them from xterm-256color, IIRC) instead of being declared explicitly. DecouplingThings I did would work both for "CPP" approach and for "vty-core/vty-unix/vty-conhost" approach. That being said, the latter approach looks more maintainable. Significant changes off the top of my head:
Other changes seem benign. |
Come to think of that, we will need to test several other terminal emulators, at least document their behaviour with respect to vty:
I guess that with some tinkering and hardcoding cygwin-based emulators should work with current vty. I should be able to carve some time for testing next week. And we will probably need a test "who's my host"? |
I'll just add some comments on what Shryke mentioned...
This one really confuses me. What do you mean by 'if the user does not use utf8'?
I believe this should be handled correctly by choosing the appropriate size of the buffers. (InputRecord buffer size = bytes buffer / 4) I set InputRecord buffer size = 256, 'byte buffer' = 1024. If there are more than 256 input records in the buffer, then it will take multiple passes to consume all the input, but there should not be any overflow. (One input char has max size of 4 bytes)
Not sure if there is anything to do here. I believe the linux implementation also ignores mouse moves (unless a button is pressed).
I ran the benchmark, but didn't really pay attention to results yet. So yeah, would be good to compare Windows to linux performance.
After some digging around other projects, it seems the most commonly used approach is to define a single library with OS specific modules and specifying which modules to build with 'if os(windows) ...' in the cabal file. I think I'm actually leaning in that direction at the moment. If we do it right, I think we should be able to avoid #ifdef pragmas. |
I had some time to do basic tests
We can detect
That being said, I am not willing to investigate the problems with these two emulators any further right now. |
This can now be closed! |
As part of understanding what Windows Terminal support in Vty would entail, we need to understand how much overlap exists between the sequences that Vty currently uses for its terminfo-based and Xterm output backends and the ones documented as supported by Windows Terminal at:
https://learn.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences
Essentially, if there is enough overlap in the core feature set needed by Vty, then it may be possible that our current implementation will "just work" in Windows Terminal. If not, the existing backends will need to be used as a basis for a Windows Terminal backend. (I suspect that for some other reasons that will be the case anyway, but it would be helpful to know how much overlap there is anyway.)
In particular, the file
src/Graphics/Vty/Output/TerminfoBased.hs
will need to be inspected to compare its sequence use against the Windows Terminal documentation. The same goes for the Xterm backend insrc/Graphics/Vty/Output/XTermColor.hs
.The text was updated successfully, but these errors were encountered: