Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Evaluate feature parity between Windows Terminal sequences and existing backends #251

Closed
jtdaugherty opened this issue Nov 17, 2022 · 48 comments

Comments

@jtdaugherty
Copy link
Owner

jtdaugherty commented Nov 17, 2022

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 in src/Graphics/Vty/Output/XTermColor.hs.

@jtdaugherty jtdaugherty added this to the Windows Terminal support milestone Nov 17, 2022
@jtdaugherty jtdaugherty changed the title Evaluate feature parity between Windows Terminal sequences and Vty Evaluate feature parity between Windows Terminal sequences and existing backends Nov 17, 2022
@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Feb 20, 2023

Is it safe to assume that if we get a positive (or almost positive answer) to this issue, and we manage to migrate Fd to Abstract-Handle as proposed in #219, then nothing is holding vty from running in windows terminal?

Relevant issue: MicrosoftDocs/Console-Docs#289

@ShrykeWindgrace
Copy link

...and we have to compare everything by hand, is there a documented list of terminal features/capabilities that are required for vty? Except examining everything by hand in the files you linked above?

@ShrykeWindgrace
Copy link

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

@jtdaugherty
Copy link
Owner Author

Is it safe to assume that if we get a positive (or almost positive answer) to this issue,

What do you mean by "positive answer"?

and we manage to migrate Fd to Abstract-Handle as proposed in #219,

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 vty-unix package and leave vty-windows to do whatever is appropriate for that platform.

then nothing is holding vty from running in windows terminal?

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!).

@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Feb 21, 2023

Is it safe to assume that if we get a positive (or almost positive answer) to this issue,

What do you mean by "positive answer"?

Sorry, I could have written that phrase in a better way. What I wanted to say was "terminal features required by vty are supported by WindowsTerminal".

and we manage to migrate Fd to Abstract-Handle as proposed in #219,

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 vty-unix package and leave vty-windows to do whatever is appropriate for that platform.

I guess the approach with separate vty-windows package would require a marge larger scope of refactoring, albeit better in the long run.

then nothing is holding vty from running in windows terminal?

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!).

Let's hope for the best and prepare for the worst. After all, vty has a plethora of terminfo and xterm terminals to be aware of, and only one WindowsTerminal =)

@jtdaugherty
Copy link
Owner Author

I guess the approach with separate vty-windows package would require a marge larger scope of refactoring

Possibly; the internal abstractions are probably already pretty close (Input and Output being the main ones). I suspect that moving to vty-core + vty-unix + vty-windows would result in the core being pretty small and most of the Unix terminal ugliness being confined to vty-unix.

After all, vty has a plethora of terminfo and xterm terminals to be aware of, and only one WindowsTerminal

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.

@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Feb 21, 2023

I hope you pardon me for a bit of bragging:
image

That's output of your vty-demo on my win10+WindowsTerminal+pwsh setup. There are many things that are not implemented, but at least I can see something other than a blank screen=) I intentionally exposed only Color16 mode.

@jtdaugherty
Copy link
Owner Author

Well, this is news to me. Have you been hacking on Vty?

@ShrykeWindgrace
Copy link

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 WindowsTerminal and vty. Nothing insurmountable, just need some time for focused googling.

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 Enter to send data to vty.

@ShrykeWindgrace
Copy link

Status report (with obligatory disclaimer it works on my machine, ymmw)

  • keyboard input works, if sometimes sluggishly; a key event is not processed until the next event. I do not know why it happens

  • getWindowSize is ok

  • FullColor support seems ok

  • I am still struggling with mouse events

  • graceful restoration of terminal settings upon exit is yet to be done

  • setWindowSize is difficult (for me); MS explicitly discourages its usage (https://learn.microsoft.com/en-us/windows/console/setconsolescreenbufferinfoex#remarks). Our rust colleagues seem to have an implementation nonetheless in e.g. crossterm

  • my code is ugly, I'll need quite some time for refactoring and extracting a proper vty-windows out of that mess

On this note, my time budget for OSS is depleted for the foreseable future, so my workings will lay dormant.

@jtdaugherty
Copy link
Owner Author

I'll need quite some time for refactoring and extracting a proper vty-windows out of that mess

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?

@ShrykeWindgrace
Copy link

Could you commit what you have on a fork and provide access to it?

Will do sometimes next week, most probably on Tuesday.

@jtdaugherty
Copy link
Owner Author

Okay, thank you!

@chhackett
Copy link

Hello Jonathan and ShrykeWindgrace, I've been doing similar hacking and have similar results.
I checked in my changes into a fork here: https://github.com/chhackett/vty

Perhaps we can compare notes, and split up the remaining investigation?

  • a few things I've noticed are:
    • Color240 mode is still broke in my version, but FullColor mode works
    • Shryke: one difference between our vty versions seems to be that you got the bold colors to work - i hadn't tried to get that working yet
    • keyboard input works nicely (i'm building with ghc 9.4.X using --io-manager=native) - you don't have to press return for key events
    • mouse events don't seem to work correctly. haven't looked into why
    • window size i think works (i don't have a good test to verify it yet)

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:
type Terminal = Terminfo.Terminal
type SetupTermError = Terminfo.SetupTermError
type NativeHandle = PosixTypes.Fd
type ByteCount = PosixTypes.ByteCount

And all the functions:
setupTerm :: String -> IO Terminal
getTermType :: Maybe String
getCapability = Terminfo.getCapability
tiGetStr = Terminfo.tiGetStr
tiGetNum = Terminfo.tiGetNum
stdInput = PosixIO.stdInput
stdOutput = PosixIO.stdOutput
readBuf :: NativeHandle -> Ptr Word8 -> ByteCount -> IO ByteCount
writeBuf :: NativeHandle -> Ptr Word8 -> ByteCount -> IO ByteCount
getTtyEraseChar :: NativeHandle -> IO (Maybe Char)
getWindowSize :: NativeHandle -> IO (Int,Int)
setWindowSize :: NativeHandle -> (Int, Int) -> IO ()
attributeControlInternal :: NativeHandle -> IO (IO (), IO ())
configureOutput :: NativeHandle -> IO ()
setNonBlockingBufferMode :: NativeHandle -> IO ()
installWindowChangeHandler :: Maybe (IO ()) -> IO ()
installContinueProcessHandler :: Maybe (IO ()) -> IO ()
installSignalHandler :: Maybe (IO ()) -> Signal -> IO ()

Look forward to working with you guys...

@noughtmare
Copy link
Contributor

noughtmare commented Feb 23, 2023

keyboard input works nicely (i'm building with ghc 9.4.X using --io-manager=native)

Sounds like it may have been caused by GHC issue #21665 which was fixed in 9.4.2.

@chhackett
Copy link

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.

@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Feb 23, 2023

RE: mouse input. That's most probably because it's disabled and we need to explicitly enable it.

In a ghci session I get

ghci> (eNABLE_MOUSE_INPUT .&.) <$> withHandleToHANDLE stdout getConsoleMode
0
ghci> (eNABLE_MOUSE_INPUT .&.) <$> withHandleToHANDLE stdin getConsoleMode
16

in other words, stdout has no mouse events.

@ShrykeWindgrace
Copy link

And another observation - I've been browsing through rust's crossterm crate for inspiration (that's, roughly speaking, their equivalent of vty/brick). Unfortunately, they do quite a lot of things via console API, not via virtual sequences, despite official recommendations. If we adopt the same approach, we will need an extensive patching of Haskell's Win32 package

@ShrykeWindgrace
Copy link

As promised, my fork, use it at your own risk =) https://github.com/ShrykeWindgrace/vty

Tested against ghc-9.4.4

  • kb input events work
  • mouse button input events work
  • mouse moves yield parser failures; I do not know whether that's by design or by mismatch unix/windows
  • FullColor and Color16 modes work; I did not try Color240 (to be honest, I have any idea how it should work)
  • most escape sequences were blindly copied from tmux-256color
  • Fd mentions were either dropped or replaced by Handle

What I did not implement and will not have time to do in the next couple of weeks:

  • "window input" and "continue process" subscriptions
  • console character encoding
  • output capabilities configuration

I still want to hack brick a bit to remove unix and terminfo related stuff, but no promises.

I'll be available to answer questions on my code on github.

Cheers!

@chhackett
Copy link

chhackett commented Mar 3, 2023

Hello @ShrykeWindgrace,
It looks like we took slightly different paths to reach our conclusions, but I have similar results as you except in a few areas where our implementations gave different results. Your implementation had a more correct set of capability definitions (props for realizing tmux has been here before us), so after shamelessly copying your capability strings, all the color modes work in my version. (I think they'd work in yours too, but you kind of removed XtermColor from your implementation)
A few things to note:

  • in my testing, I compared behavior on Windows to behavior in a WSL2 terminal. (my implementation compiles on both linux and windows) This helped me to spot differences in behavior.
  • it isn't necessary to use eNABLE_MOUSE_INPUT to get mouse input via VT sequences. That is just for using the old Win32 api to read the mouse events. As long as VT mode is enabled, when the system sets the mouse mode with one of the special sequences (see Mouse.hs) the terminal will receive the mouse events. I got that working in my version after a little bug fix (the input polling loop needs a little delay in between reading the input buffer. i was probably reading that buffer a million times a second). So now mouse modes work in my impl.
  • Windows resize support: I got resize events working on Windows by polling the size in a separate thread. We might want to manage the thread in a better way though.
  • As for 'console character encoding' I noticed that you don't build 'interactive_terminal_test'. That turned out to be an exceptionally useful utility that verifies support for UNICODE characters in addition to all the color support, cursor modes, etc. In my version, all those tests passed, so UNICODE works. I suspect it should work in yours too.
    Here's a snippet from one test:
    你好吗
    012345

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

@ShrykeWindgrace
Copy link

@chhackett Thanks for the comprehensive review =)

(my implementation compiles on both linux and windows)

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 =( )

it isn't necessary to use eNABLE_MOUSE_INPUT to get mouse input via VT sequences

Good to know. Does your implementation handle mouse moves? Or only mouse buttons?

I got resize events working on Windows by polling the size in a separate thread

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.

UNICODE works. I suspect it should work in yours too.

Quite probably. Since my terminal/shell is set with, essentially, chcp 65001, I did not explicitly test for that, deeming that a lower priority.

@ShrykeWindgrace
Copy link

On a side note, I tried compiling hledger-iadd, it uses the shortcut Ctrl+z for undo, that's how I spotted the bug.

I am inclined to add the special treatment for the case "input available, but of length zero" and manually send BS.pack [26] in that case. In my local tests that works, but it is possible that I do not see the whole picture.

@ShrykeWindgrace
Copy link

And a couple of questions:

  • since both @chhackett and I managed to whip up something working (I already started dogfooding my forks of vty and brick), can we (re)start a discussion on the architectural approach? Do we split vty into vty-core/vty-unix/vty-windows? Or do we spruce the code with CPP and go with AbstractHandle? Or something else?
  • it seems that we can start opening issues for individual windows quirks and idiosyncrasies - they will be there regardless of the chosen approach
  • if there is a need for IM-like channels of communication (IRC/Telegram/Discord/whatever), I'm available during work hours of CET/CEST (UTC+1/+2).

@chhackett
Copy link

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 =( )

Oh, yeah, I had similar kinds of problems mostly with my work laptop because they lock down features. It sucks.

Good to know. Does your implementation handle mouse moves? Or only mouse buttons?

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)

I got resize events working on Windows by polling the size in a separate thread

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.

Nope, don't receive events, just poll for them. It would be wonderful if we could get VT sequences for window resizes...
Anyway, I just call getConsoleScreenBufferInfo in a separate thread (5 times a sec for now), and run the 'pokeIO' action when the size changes.

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?
-- Not sure if there is a reason to manage killing the thread on shutdown, or let GHC do it.
installWindowChangeHandler :: Maybe (IO ()) -> IO ()
installWindowChangeHandler mHandler = do
case mHandler of
Just handler -> do
-- logWindowChange "Registering for window change events"
_ <- forkOS $ runWindowChangeLoop handler
return ()
Nothing -> logWindowChange "Unregistering for window change events"

runWindowChangeLoop :: IO () -> IO ()
runWindowChangeLoop action = do
s0 <- getWindowSize stdOutput
evalStateT (windowChangeLoop action) s0

windowChangeLoop :: IO () -> WindowChangeStateM ()
windowChangeLoop action = do
liftIO $ threadDelay 200000 -- we will check 5 times/sec for now. should be faster in production i suppose
(x, y) <- get
(x', y') <- liftIO $ getWindowSize stdOutput
if x /= x' || y /= y'
then do
liftIO $ logWindowChange $ "Got new size: " ++ show (x', y')
liftIO action
put (x', y')
windowChangeLoop action
else windowChangeLoop action

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.

@ShrykeWindgrace
Copy link

On setWindowSize/setDisplayBounds:

in Windows Terminal this could be problematic: microsoft/terminal#4417 and microsoft/terminal#5094 off the top of my head google.

I"ll try to test the reception of window resize events (VT or anything else) - not earlier than next Monday, though.

@chhackett
Copy link

On a side note, I tried compiling hledger-iadd, it uses the shortcut Ctrl+z for undo, that's how I spotted the bug.

I am inclined to add the special treatment for the case "input available, but of length zero" and manually send BS.pack [26] in that case. In my local tests that works, but it is possible that I do not see the whole picture.

Good catch! I tried it and it totally freezes the console window. Nothing to do then but kill the window, lol.
Also noticed Ctrl-D does the same thing, which makes sense since that is supposed to send EOF according to docs.
Sounds like we can detect the scenario and work around it, so that's good at least.

@j4james
Copy link

j4james commented Mar 4, 2023

Windows resize support: I got resize events working on Windows by polling the size in a separate thread.

I think you can probably do this more efficiently by waiting for a WINDOW_BUFFER_SIZE_EVENT. See the documentation here: https://learn.microsoft.com/en-us/windows/console/reading-input-buffer-events

@ShrykeWindgrace
Copy link

Windows resize support: I got resize events working on Windows by polling the size in a separate thread.

I think you can probably do this more efficiently by waiting for a WINDOW_BUFFER_SIZE_EVENT. See the documentation here: https://learn.microsoft.com/en-us/windows/console/reading-input-buffer-events

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.

@chhackett
Copy link

I think you can probably do this more efficiently by waiting for a WINDOW_BUFFER_SIZE_EVENT. See the documentation here: https://learn.microsoft.com/en-us/windows/console/reading-input-buffer-events

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.

@ShrykeWindgrace
Copy link

@ShrykeWindgrace
Copy link

Two immediately useful pieces of info:

@chhackett
Copy link

chhackett commented Mar 12, 2023

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:

  • Consume all input via ReadConsoleInputW.
  • Consume key/mouse events via VT sequences (as we do now), and poll for window resize events.

Solution 1 Pros:

  • We no longer need to set eNABLE_VIRTUAL_TERMINAL_INPUT... all events are processed by ReadConsoleInputW
  • Parsing input events is a lot simpler. All events are received as a C struct that is parsed into simple haskell data types:
data WinConsoleInputEvent =
    KeyEventRecordU KeyEventRecord
  | MouseEventRecordU MouseEventRecord
  | WindowBufferSizeRecordU WindowBufferSizeRecord
  | MenuEventRecordU MenuEventRecord
  | FocusEventRecordU FocusEventRecord

Solution 1 Cons:

  • Separate input processing means maintainability suffers.

Solution 2 Pros:

  • The windows/posix libraries get to utilize a common input processing pipeline.

Solution 2 Cons:

  • We have a thread constantly polling for window resize events.

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

@chhackett
Copy link

@jtdaugherty @ShrykeWindgrace @noughtmare
My summary so far:

  • sequence use in TerminfoBased and XTermColor is easy. When we supply the correct sequences, it just works. That takes care of the output side.
  • The input side is harder purely because of the platform dependent mechanisms for handling window resize. Posix uses signals, while Windows has its own special APIs (multiple ways of doing it on Windows)

@ShrykeWindgrace
Copy link

@chhackett I got quite the opposite impression/approach to the solution 1'
As far as my testing goes:

  • we set ENABLE_VIRTUAL_TERMINAL_INPUT with other flags to get mouse and windows events
  • we consume input as ReadConsoleInput
  • the KEY_EVENT_RECORD will contain a bit of noise, but we can easily filter that noise out:
    • key releases are bogus, drop them
    • key presses with ker.uChar.AsciiChar > 0 represent our VT sequences. Keep only the AsciiChar part, other fields can be dropped (at least for now).

Again, to my understanding, this fits in the loop part (parseEvent and readFromDevice) rather nicely. After all, that part listens to input and produces Events.

For example readFromDevice would produce Either Event ByteString, with Left variant being resize events and Right being fed to the parser. The changes are rather isolated.

I'll try to make the necessary bindings next week, but anyone already has them, I'll be grateful=)

@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Mar 13, 2023

I am logging events that I receive via ReadConsoleInput (focus enabled, mouse buttons enabled, window enabled, mouse move disabled)

output mode 7
try to request alternate buffer, focus events, disable mouse_any and enable mouse_btn 
iteration 1
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 13, virtualScanCode = 28, unicodeChar = '\r', controlKeyState = 32}
WindowEvent {dwSize_ = COORD {xPos = 209, yPos = 55}}
iteration 2
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\ESC', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '[', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = 'O', controlKeyState = 0}
iteration 3
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\ESC', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '[', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '<', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '0', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = ';', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '5', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '6', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = ';', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '2', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '0', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = 'M', controlKeyState = 0}
iteration 4
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\ESC', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '[', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = 'I', controlKeyState = 0}
iteration 5
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\ESC', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '[', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '<', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '0', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = ';', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '5', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '6', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = ';', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '2', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '0', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = 'm', controlKeyState = 0}
iteration 6
WindowEvent {dwSize_ = COORD {xPos = 209, yPos = 59}}
iteration 7
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 122, virtualScanCode = 87, unicodeChar = '\NUL', controlKeyState = 32}
iteration 8
WindowEvent {dwSize_ = COORD {xPos = 209, yPos = 55}}
iteration 9
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 122, virtualScanCode = 87, unicodeChar = '\NUL', controlKeyState = 32}
iteration 10
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\r', controlKeyState = 0}
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 13, virtualScanCode = 28, unicodeChar = '\r', controlKeyState = 32}
iteration 11
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\DEL', controlKeyState = 0}
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 8, virtualScanCode = 14, unicodeChar = '\b', controlKeyState = 32}
iteration 12
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 17, virtualScanCode = 29, unicodeChar = '\NUL', controlKeyState = 292}
iteration 13
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 17, virtualScanCode = 29, unicodeChar = '\NUL', controlKeyState = 288}
iteration 14
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\t', controlKeyState = 0}
iteration 15
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 9, virtualScanCode = 15, unicodeChar = '\t', controlKeyState = 32}
iteration 16
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\ESC', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '[', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = 'D', controlKeyState = 0}
iteration 17
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 37, virtualScanCode = 75, unicodeChar = '\NUL', controlKeyState = 288}
iteration 18
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\ESC', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '[', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = 'C', controlKeyState = 0}
iteration 19
KeyEvent {keyDown = False, repeatCount = 1, virtualKeyCode = 39, virtualScanCode = 77, unicodeChar = '\NUL', controlKeyState = 288}
iteration 20
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '\ESC', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = '[', controlKeyState = 0}
KeyEvent {keyDown = True, repeatCount = 1, virtualKeyCode = 0, virtualScanCode = 0, unicodeChar = 'A', controlKeyState = 0}

I we squeeze hard enough on the unicodeChar field, we see focus events \ESC[O and \ESC[I, enter represented as \r, mouse clicks, window resizing, arrow presses (e.g. \ESC[D, tabs and backspaces... The \NUL entries represent taps on control buttons (ctlr, shift,...). If we ever need to distinguish them individually, we'll need to look at controlKeyState.

Basing on these logs, I am quite confident that ReadConsoleInput with ENABLE_VIRTUAL_TERMINAL_INPUT is a viable option worth exploring.

@ShrykeWindgrace
Copy link

Yet another argument against reading raw bytes from haskell's stdin. For non-ASCII input that would fail:
https://gitlab.haskell.org/ghc/ghc/-/issues/23133

And the root cause:
microsoft/terminal#11956 (comment)

The console API doesn't support reading input as UTF-8

@chhackett
Copy link

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.

@ShrykeWindgrace
Copy link

I am not sure about re-encode character input - we do that anyways (readDevice just fetches bytes from stdin and packs them into a ByteString, then we concatenate these bytestrings and parse them).

In my tests I recover at most one haskell's Char per InputEvent - that's the intended behavior of ReadConsoleInput; the process remains virtually the same: concatenate these Chars, then parse. Nothing stops me from reading multiple InputEvents at once, however. With the 150ms delay and my speed of typing I get around a dozen of events per batch.

I have no other ideas right now that would allows us (at the same time):

  • to fetch multiple significant Bytes/Chars at once without fetching auxiliary info
  • work with non-ASCII input
  • receive window resize events without a separate polling thread

I'll commit my implementation by the end of next Tuesday (CET). Monday, if my free time allows for that=)

@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Mar 20, 2023

@chhackett @jtdaugherty as promised, I pushed to my fork of vty the implementation with ReadConsoleInput. If fails on terminal emulators like MobaXterm (for obvious reasons), but in WT it works nicely. There is still quite a lot of things that we can do better, in terms of allocations/parser/feature queries/etc. And, most notably, I still need to interleave input events with window resize events.

link: https://github.com/ShrykeWindgrace/vty

@chhackett
Copy link

@ShrykeWindgrace
Just FYI, I discovered this morning that there is already a very nice implementation of the entire Win32 console API in the aptly named Win32-console package. I now understand why the console API wasn't already added to the Win32 base package.

@ShrykeWindgrace
Copy link

@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 WaitForSingleObject, so I decided to roll out my own bindings.

@chhackett
Copy link

@ShrykeWindgrace
So my prototype using ReadConsoleInputW is checked in. It works for all unicode characters btw. And my impl handles all the events (focus, resize, etc). I did notice that the ReadConsoleInputW foreign call seems to block all threads (including main thread). Which it shouldn't because it is called a separate thread (using forkOS) and everything is built with -threaded. So I copied your fix using the waitForSingleObject + yield. I just don't understand why that is necessary. I wonder if its possible this is a WinIO bug? It could also be that I don't really understand the documentation.

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.

@jtdaugherty
Copy link
Owner Author

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

  • What's left that isn't working at all? Or is working, but not quite well enough?
  • Did you need to put in anything gross (in your view) to make things work, that might ultimately need some attention in a cleaner implementation?
  • Did your work bring to mind any suggestions for how to make Vty less coupled to Unix idioms and be made to be generic enough to work for Windows?

@ShrykeWindgrace
Copy link

@chhackett @jtdaugherty

Missing

What is missing in my fork:

  • minor things:

    • setting and resetting console input/output options if the user does not use utf8
    • on-the-fly handling of the case where user inputs too fast and the pre-allocated buffer for the events overflows
    • interactive-terminal-test has one failing test with text styling
    • one HANDLE allocation per input read, we can definitely do better
    • quering the terminal for enabled/disabled reporting features (I prefer to disable mouse moves, they give too much noise); also blocked by release timeline of windows terminal, the queries are available only in the preview release
    • performance/throughput untested
  • major things:

    • interleaving user input with window resize events in the loop (I have not had the time to check @chhackett 's fork for that feature)
    • bracketed paste untested
    • detecting that we are running in conhost. If I run my app in, say, MobaXTerm (is it cygwin?), the app fails with a rather unhelpful message.
    • (probably not that relevant) one of the widgets in brick still uses unix.
    • setWindowSize - not implemented

Otherwise, my daily driver brick+vty CLI apps work as intended.

Gross/ugly

VT sequences for terminal capabilities are parsed (I copied them from xterm-256color, IIRC) instead of being declared explicitly.
Other than that, the code has only minor hiccups.

Decoupling

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

  • handle and HANDLE (win32 equivalent); FD is no more
  • readFromDevice, InputBuffer, InputState and their friends changed their signatures (the read happens via hsc bindings and it receives window resize events)
  • Config now should hold both handle and HANDLE instead of one FD for their input and output
  • attributeControl uses Config instead of FD/Handle as argument

Other changes seem benign.

@ShrykeWindgrace
Copy link

Come to think of that, we will need to test several other terminal emulators, at least document their behaviour with respect to vty:

  • alacritty
  • mobaXTerm
  • conemu
  • cmder
  • hyper
  • Xshell
  • putty
  • mintty
  • ...
    Some of them could very well be just another frontend for conhost, some are cygwin-based, some could be a different beast entirely.

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"?

@chhackett
Copy link

chhackett commented Mar 27, 2023

I'll just add some comments on what Shryke mentioned...

  • setting and resetting console input/output options if the user does not use utf8

This one really confuses me. What do you mean by 'if the user does not use utf8'?

  • on-the-fly handling of the case where user inputs too fast and the pre-allocated buffer for the events overflows

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)

  • interactive-terminal-test has one failing test with text styling
  • one HANDLE allocation per input read, we can definitely do better
    Yup, agreed. We should store the HANDLE...
  • quering the terminal for enabled/disabled reporting features (I prefer to disable mouse moves, they give too much noise); also blocked by release timeline of windows terminal, the queries are available only in the preview release

Not sure if there is anything to do here. I believe the linux implementation also ignores mouse moves (unless a button is pressed).

  • performance/throughput untested

I ran the benchmark, but didn't really pay attention to results yet. So yeah, would be good to compare Windows to linux performance.

  • interleaving user input with window resize events in the loop (I have not had the time to check @chhackett 's fork for that feature)
    Yeah, that's handled in my prototype.
  • bracketed paste untested
    I tested it. It works.
  • detecting that we are running in conhost. If I run my app in, say, MobaXTerm (is it cygwin?), the app fails with a rather unhelpful message.
    Hmm, I guess I could imagine failing with a better error message? Not sure what the expectation would be for trying to run a Windows console app in a non-Windows terminal.
  • setWindowSize - not implemented
    I have an implementation, but haven't tested it (examples don't exercise that feature). Don't even know the expected behavior. Should the terminal window itself actually grow/shrink?

Decoupling

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

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.

@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Mar 31, 2023

I had some time to do basic tests alacritty and ConEmu.

alacritty is a frontend to conhost of sorts, but it pretends to be an xterm. Most of the things work out of the box, except for

  • blinking text
  • underlined text
  • the last test in the "iteractive terminal test", it has some jitter/extra characters in the bottom of the screen.

We can detect alacritty by the presence of an environment variable ALACRITTY_LOG.

ConEmu also seems to be a frontend to conhost (I am 95% sure), but with a lot of bells, whistles, DLL injecting, and low-level stuff in its configuration going on. With default config the rendered UI looks somewhat ugly, there extra symbols being rendered. And the major problem (fixable, I hope) - some VT sequence arrive cut in two. For example, the sequence for an arrow \ESC[A arrives in two chunks, [\ESC[, A], hence the parser falls back to recognizing only \ESC, the consecutive characters are seen as regular input.
We can detect ConEmu but its plethora of environment variables ConEmuSomething.

That being said, I am not willing to investigate the problems with these two emulators any further right now.

@jtdaugherty
Copy link
Owner Author

This can now be closed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants