-
Notifications
You must be signed in to change notification settings - Fork 721
Fixes #4221 Extra modifiers f1 to f4 in v2net #4220
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
base: v2_develop
Are you sure you want to change the base?
Conversation
…lt unless user explicitly forces 16
…by default unless user explicitly forces 16" This reverts commit 4cc2530.
@@ -25,6 +25,7 @@ internal class MainLoopCoordinator<T> : IMainLoopCoordinator | |||
private ConsoleDriverFacade<T> _facade; | |||
private Task _inputTask; | |||
private readonly ITimedEvents _timedEvents; | |||
private readonly bool _isWindowsTerminal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to check if the console support VTS. In this case the field should be _isVirtualTerminal
. Of course with v2win you have to get this from the STD_OUTPUT_HANDLE and verify if ENABLE_VIRTUAL_TERMINAL_PROCESSING is enabled. Alacritty has VTS enabled and if you only check for Environment.GetEnvironmentVariable ("WT_SESSION") is { } || Environment.GetEnvironmentVariable ("VSAPPIDNAME") != null;
it won't support true colors because you'll force 16 colors. With v2net VTS is always enabled by itself, so true color is always true in Unix like system and in Windows depends of the build version. The problem is check for VTS in this class. On my PR that I reverted I did a call to WindowsOutput to get the internal field IsVirtualTerminal
, but you probably have another better solution for this. In Windows also always check the OS version build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not in diff I think, I revert that accidental commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not in diff I think, I revert that accidental commit
I saw again and it was reverted, sorry. Good choice to assume they assume true colors at start. After start the app they are assumed or not accordingly with what the console supports.
Fixes
Proposed Changes/Todos
F1 key can be
<esc>OP
But if combined with modifier keys it can instead be:
<esc>[1;3P
This PR updates key parsing to support this sequence
Pull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commit///
style comments)