Add support for the S8C1T/S7C1T escape sequences #17945
Conversation
microsoft-github-policy-service
bot
added
the
Issue-Feature
microsoft-github-policy-service
bot
added
Area-VT
src/host/outputStream.cpp
Outdated
| if (IsValidCodePage(codepage)) | ||
| { |
There was a problem hiding this comment.
Is there a reason why we should check this outside the two set functions? I think it may be safer if it was inside them.
There was a problem hiding this comment.
It just seemed a little more efficient, but that's not really important here. I've moved the checks back into the DoSrvSet* functions now.
| const auto codepage = _api.GetConsoleOutputCP(); | ||
| const auto codepage = _api.GetOutputCodePage(); | ||
| InputEventQueue keyEvents; |
There was a problem hiding this comment.
I forgot to mention that I noticed we're using the output code page here to generate input events, which seemed wrong to me. Was that intentional?
That codepage is only used if the UseNumpadEventsForClipboardInput feature is enabled (which doesn't appear to be the case), so I don't think it's breaking anything at the moment, but it's still worth fixing if it's incorrect.
There was a problem hiding this comment.
That's an excellent question. Personally I'm not sure, though I suspect that it was an oversight.
UseNumpadEvents... was a last minute "don't break Windows conhost users right before a big release" feature flag, which we should almost certainly just remove now... :)
| _api.ReturnResponse(fmt::format(FMT_COMPILE(L"{}{}{}"), dcs, response, st)); | ||
| } | ||
|
|
||
| void AdaptDispatch::_ReturnOscResponse(const std::wstring_view response) const |
There was a problem hiding this comment.
Thought: Xterm specifies that OSC responses are returned with the same terminator (ST or BEL) as the request that generated them. I suppose having this function affords us a much easier way to find/fix all occurrences of us being out of spec :)
XTerm accepts either BEL or ST for terminating OSC sequences, and when returning information, uses the same terminator used in a query. While the latter is preferred, the former is supported for legacy applications: o Although documented in the changes for X.V10R4 (December 1986), BEL as a string terminator dates from X11R4 (December 1989). o Since XFree86-3.1.2Ee (August 1996), xterm has accepted ST (the documented string terminator in ECMA-48).
There was a problem hiding this comment.
I really hate that BEL terminator, but you're right, we should be responding with BEL if the query was terminated with BEL.
lhecker
added
the
AutoMerge
microsoft-github-policy-service
bot
merged commit aa256ad
into
microsoft:main
This PR adds support for the
S8C1TandS7C1Tcommands, which enablean application to choose whether the terminal should use C1 controls
when sending key sequences and query responses.
This also updates the
DOCScommand to set both the input and outputcode pages. So when switched to ISO2022 mode, the C1 controls will be
transmitted as 8-bit, which is what legacy systems would be expecting.
Detailed Description of the Pull Request / Additional comments
While adding the input code page support, I also reworked the way we
handle the code page reset in
RIS. In the original implementation wesaved the active code page when the
DOCSsequence was first used, andthat would become the default value for a reset.
With this PR I'm now saving the code pages whenever
SetConsoleCPorSetConsoleOutputCPis called, so those APIs now control what thedefault values will be. This feels more consistent than the previous
approach. And this is how WSL sets its initial code page to UTF-8.
Validation Steps Performed
I've added a couple of unit tests that check one of each applicable C1
control in the key sequences and query reports.
I also built myself a code page aware telnet client so I could log into
WSL in 8-bit mode, and confirmed that the C1 transmissions are working
as expected in vttest.
Closes #17931
Tests added/passed