jtdaugherty / vty

A high-level ncurses alternative written in Haskell
BSD 3-Clause "New" or "Revised" License
319 stars 57 forks source link

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

Closed jtdaugherty closed 10 months ago

jtdaugherty commented 1 year ago

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.

ShrykeWindgrace commented 1 year ago

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: https://github.com/MicrosoftDocs/Console-Docs/issues/289

ShrykeWindgrace commented 1 year ago

...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 commented 1 year ago

A bit of digging on bracketed paste: it kinda works, according to https://github.com/microsoft/terminal/issues/395; most notably https://github.com/microsoft/terminal/issues/395#issuecomment-778707270 and https://github.com/microsoft/terminal/pull/9034

jtdaugherty commented 1 year ago

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 https://github.com/jtdaugherty/vty/issues/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

ShrykeWindgrace commented 1 year ago

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 https://github.com/jtdaugherty/vty/issues/219#issuecomment-1139945890

Major limitation for now - user input does not work as it should, I need to press Enter to send data to vty.

ShrykeWindgrace commented 1 year ago

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

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

jtdaugherty commented 1 year ago

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 commented 1 year ago

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

Will do sometimes next week, most probably on Tuesday.

jtdaugherty commented 1 year ago

Okay, thank you!

chhackett commented 1 year ago

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?

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 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 commented 1 year ago

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:

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

And a couple of questions:

chhackett commented 1 year ago

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 commented 1 year ago

On setWindowSize/setDisplayBounds:

in Windows Terminal this could be problematic: https://github.com/microsoft/terminal/issues/4417 and https://github.com/microsoft/terminal/issues/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Rather outdated, but possibly useful: https://terminalnuget.blob.core.windows.net/packages/TerminalSequences.html#window-management

ShrykeWindgrace commented 1 year ago

Two immediately useful pieces of info:

chhackett commented 1 year ago

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

chhackett commented 1 year ago

@jtdaugherty @ShrykeWindgrace @noughtmare My summary so far:

ShrykeWindgrace commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/microsoft/terminal/issues/11956#issuecomment-999482461

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

chhackett commented 1 year ago

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 commented 1 year ago

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

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

ShrykeWindgrace commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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

ShrykeWindgrace commented 1 year ago

@chhackett @jtdaugherty

Missing

What is missing in my fork:

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:

Other changes seem benign.

ShrykeWindgrace commented 1 year ago

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

chhackett commented 1 year ago

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 commented 1 year ago

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

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 commented 10 months ago

This can now be closed!