PowerShellEditorServices
PowerShellEditorServices copied to clipboard
Added a -UseHostReadKey switch to instruct LegacyReadLine to use the …
…ReadKey provided by the host instead of Console.ReadKey
I removed ConsoleProxy because the static constructor was making it difficult to use PSES in a web server where everything is hosted in one process.
I moved the operations of ConsoleProxy directly into EditorServicesConsolePSHostUserInterface.cs since that was the only place it appears to be used, and then added -UseHostReadKey which prevents the instantiation of _consoleOperations, leaving it null.
I then added null checks to all usages of _consoleOperations with fallbacks to underlyingHostUI. Most importantly for ReadKey as this was famously hardcoded to Console.ReadKey under the assumption that this library would never be used outside of a local terminal.
I feel like this fits rather nicely for people like me who have taken the time to properly implement a custom PSHostRawUserInterface
Reviewing your PR has me noticing that ReadKeyAsync
is implemented...yet never used. @dkattan a few issues I'm investigating seem to be caused by our override of ReadKey
(for PSReadLine): https://github.com/PowerShell/vscode-powershell/issues/3881, https://github.com/PowerShell/vscode-powershell/issues/2741, and https://github.com/PowerShell/vscode-powershell/issues/3876. So...I'm very intrigued here because I'm already working in this area and trying to come up with a better solution than "gobble everything and pretend we didn't."
Reviewing your PR has me noticing that
ReadKeyAsync
is implemented...yet never used. @dkattan a few issues I'm investigating seem to be caused by our override ofReadKey
(for PSReadLine): PowerShell/vscode-powershell#3881, PowerShell/vscode-powershell#2741, and PowerShell/vscode-powershell#3876. So...I'm very intrigued here because I'm already working in this area and trying to come up with a better solution than "gobble everything and pretend we didn't."
I have so many questions.
- Why do we need that override in the first place?
- Why does PSReadLine use System.Console.ReadKey() instead of PSHostRawUserInterface.ReadKey?
- Why do we need that override in the first place?
Two reasons, broken up by platform:
On Windows it's so we can make ReadKey
cancellable, sorta. It's a hack since there isn't any real support for cancellation but we put it in another thread and store the result in a field if it's cancelled. That way it doesn't block, and when we cancel it we can reuse the result next invocation.
On non-Windows it's due to an internal stdin lock in the BCL. If one thread is blocking on Console.ReadKey
and another thread calls Console.CursorTop
(or similar) both threads will block until input is received. PSRL internally checks cursor position after processing events to see if it needs to redraw due to host output from an event subscriber. In practice this means that Console.ReadKey
becomes synchronous and we lose the ability to cancel it.
- Why does PSReadLine use System.Console.RawUI.ReadKey() instead of PSHostRawUserInterface.ReadKey?
Assuming you mean System.Console.ReadKey()
for the former. The latter just works quite differently depending on the version, and platform where ReadKey
is pretty predictable. Every time I've tried to use RUI.ReadKey
I've found some issue with it that makes it less desirable.
Also tbh PSRL really only works in a console based host, so using the host API is typically not very important. If the host isn't console based, it's probably better for PSRL to throw.
Every time I've tried to use
RUI.ReadKey
I've found some issue with it that makes it less desirable.
I'm curious what these issues were and if they still exist. I hate to harp on it, I'm just trying to find a way to make a REPL loop work reliably in the browser in front of a net 6.0 backend.
Every time I've tried to use
RUI.ReadKey
I've found some issue with it that makes it less desirable.I'm curious what these issues were and if they still exist. I hate to harp on it, I'm just trying to find a way to make a REPL loop work reliably in the browser in front of a net 6.0 backend.
I'd also like to know why this doesn't work.
Couldn't remember so I went to go test some things, and the biggest blocker I found pretty quickly is that shift
on it's own returns as a key press (on Windows).
It's technically correct, but not the behavior we need. Technically we could just ignore those but it's a lot of extra logic and I'm not really sure what it buys us.
I can keep lookin' if y'all want but it's a super different API that we're going to end up just trying to get working like Console.ReadKey
already does.
Really, this is why we had a different host that was not terminal focused. It was torn out recently because it wasn't referenced by anything, but that should probably just be brought back with an extra switch.
Another question, you really want a custom ReadKey
? Or do you want a custom ReadLine
?
My end game is to have xterm.js in the browser with tab completion support, so I'm pretty sure I need ReadKey.
I wouldn't be opposed to my ReadKey being called after your buffering logic, I think I stand to save myself a lot of work if you guys or PSReadline was handling the buffering/cancellation/etc.
PSReadline is going to take more work because they have a singleton for clipboard purposes and that breaks my ability to have multiple consoles as when new consoles get spawned they get control of the previous instance and start sharing output.
In the interim if I could just LegacyReadline to honor my hosts ReadKey, I think I'd be set.
So sorry @dkattan, I rewrote the readkey implementation, you'll need to redo this BUT it should be way, way easier. Thank you for the inspiration.
@dkattan did you want to redo this PR or should I close it? Really sorry about the re-work I caused you, BUT hey, it's so much simpler now, no?!
@dkattan did you want to redo this PR or should I close it? Really sorry about the re-work I caused you, BUT hey, it's so much simpler now, no?!
I definitely do, just been a bit busy. I'll see if I can rebase it this afternoon.
@dkattan I'm totally still willing to take the change in spirit, it just needs to be re-written. Let me know if you get to it!