win,tty: win10 supports utf-16 surrogate pairs #2910

bnoordhuis · 2020-07-10T08:28:14Z

Fixes: #2909
CI: https://ci.nodejs.org/job/libuv-test-commit/1944/

richardlau · 2020-07-10T10:18:34Z

src/win/winapi.c

+    memset(&info, 0, sizeof(info));
+    info.dwOSVersionInfoSize = sizeof(info);
+    pRtlGetVersion(&info);
+    is_windows_10_or_greater = (info.dwMajorVersion >= 10);


Do we need a fallback setting for is_windows_10_or_greater when pRtlGetVersion == NULL?

No, because in that case is_windows_10_or_greater == 0 because it's in static storage (variables in bss are zero-initialized.)

vtjnash

This code seems arranged strangely to me, if I'm reading correctly, this would simply drop any character entirely on Windows 10 if the utf8_codepoint > 0xffff. Have you tested this visually to confirm it doesn't actually do that?

Did Microsoft give a statement or documentation update that shows when this changed? It's been a couple years since I tested to show it was still ~~MBS~~ WCS-2, so happy to hear that's been addressed now! Are these always interpreted as UTF16 now, or only in certain terminal modes?

It was perhaps a bit odd to have here special handling for conhost.exe, when that's not the only terminal emulator that could be reading this output (could also be ConEmu, Console2, etc.). Though the old behavior was attempting to play to the lowest common denominator, even then the wcswidth is sometimes wrong (true across all platforms and terminal emulators), so I'd possibly propose we eliminate this conditional entirely.

erw7 · 2020-07-10T17:55:40Z

Windows 10 version 1909 (OS build 18363.900) conhost.exe does not support the display of surrogate pairs. But Windows Terminal supports it (I don't know which version it is supported from). I believe that conhost.exe only supported it from Windows10 version 2004. It may be better for the program to guess the terminal and switch it as in uv_tty_set_vterm_state.

The following changes should also be required to display characters above U+FFFF.

--- a/src/win/tty.c
+++ b/src/win/tty.c
@@ -2857,6 +2854,12 @@ static int uv_tty_write_bufs(uv_tty_t* handle,
         ENSURE_BUFFER_SPACE(1);
         utf16_buf[utf16_buf_used++] = (WCHAR) utf8_codepoint;
         previous_eol = 0;
+      } else {
+        ENSURE_BUFFER_SPACE(2);
+        utf8_codepoint -= 0x10000;
+        utf16_buf[utf16_buf_used++] = (WCHAR) (utf8_codepoint / 0x400 + 0xD800);
+        utf16_buf[utf16_buf_used++] = (WCHAR) (utf8_codepoint % 0x400 + 0xDC00);
+        previous_eol = 0;
       }
     }
   }

DHowett · 2020-07-10T20:34:09Z

So - hi, conhost's owner here! - I definitely misread our history when I reported #2909. I'm really sorry about that!

I do, however, think the best course of action is to simply let surrogate pairs through. The console supports storing them all the way back to 10.0.14393 (the earliest version I have in a VM) -- it just doesn't support rendering them. I have the feeling that it might support storing them down to 10.0.10240, and I can verify that with an investment of about 30 minutes, which I'm happy to do on Monday.

Now, why does it matter if we store them if we can't render them? Because, as @vtjnash brings up, there are other terminal emulators on Windows. ConEmu/Cmder/ConsoleZ and WinPTY are wrappers around conhost. They're implemented as screen-scraping clients that hook in as console applications and read the buffer back. Because of that, they would still be able to benefit from having surrogate pair halves in the buffer... and they could render them "correctly". 😄

bnoordhuis · 2020-07-11T10:33:23Z

@libuv/windows Dustin's rationale sounds reasonable to me, WDYT? I'll include @erw7's patch if there's consensus.

bnoordhuis · 2020-07-20T09:36:36Z

Patch included + rebased. New CI: https://ci.nodejs.org/job/libuv-test-commit/1950/

vtjnash · 2020-07-20T20:12:39Z

src/win/tty.c

+       * console before Windows 10 doesn't really support UTF-16, so just emit
+       * the replacement character. */
+      if (!is_windows_10_or_greater && utf8_codepoint > 0xffff) {
        utf8_codepoint = UNICODE_REPLACEMENT_CHARACTER;
      }


Suggested change

* console before Windows 10 doesn't really support UTF-16, so just emit

* the replacement character. */

if (!is_windows_10_or_greater && utf8_codepoint > 0xffff) {

utf8_codepoint = UNICODE_REPLACEMENT_CHARACTER;

}

The proposal by Dustin and myself is to also eliminate this restriction entirely.

The question is, does it make sense to pass them through below windows 10? Dustin suggested that it works all the way back to

10.0.14393 (the earliest version I have in a VM)

Yes, that's just the version number for the OS he was running, but it's always been possible to run an arbitrary application on the OS which support this.

musm · 2020-08-24T04:08:10Z

bump

bnoordhuis · 2020-08-24T10:05:21Z

#2910 (comment) is still an unresolved question 🤷

musm · 2020-08-30T19:47:27Z

@bnoordhuis We have an answer #2971 (comment) , which includes the fix as suggested by conhost's owner.

bnoordhuis · 2020-09-01T19:20:36Z

Closing in favor of #2971.

win,tty: win10 supports utf-16 surrogate pairs

16de20d

Fixes: libuv#2909

bnoordhuis mentioned this pull request Jul 10, 2020

The windows console does support UTF-16 surrogate pairs now #2909

Closed

richardlau reviewed Jul 10, 2020

View reviewed changes

richardlau approved these changes Jul 10, 2020

View reviewed changes

santigimeno approved these changes Jul 10, 2020

View reviewed changes

cjihrig approved these changes Jul 10, 2020

View reviewed changes

vtjnash reviewed Jul 10, 2020

View reviewed changes

erw7 mentioned this pull request Jul 10, 2020

win,tty: Change to account for surrogate pairs neovim/libuv#13

Closed

musm mentioned this pull request Jul 16, 2020

Backport win,tty fixes for Windows systems JuliaLang/libuv#11

Merged

fixup! win10 supports utf-16 surrogate pairs

4e3ec66

vtjnash reviewed Jul 20, 2020

View reviewed changes

musm mentioned this pull request Aug 6, 2020

Release proposal: v1.39.0 #2943

Closed

musm mentioned this pull request Aug 26, 2020

win,tty: allow utf-16 surrogate pairs to pass through #2971

Closed

bnoordhuis closed this Sep 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

win,tty: win10 supports utf-16 surrogate pairs #2910

win,tty: win10 supports utf-16 surrogate pairs #2910

bnoordhuis commented Jul 10, 2020

richardlau Jul 10, 2020

bnoordhuis Jul 10, 2020

vtjnash left a comment •

edited

erw7 commented Jul 10, 2020

DHowett commented Jul 10, 2020 •

edited

bnoordhuis commented Jul 11, 2020

bnoordhuis commented Jul 20, 2020

vtjnash Jul 20, 2020

musm Aug 4, 2020

vtjnash Aug 4, 2020

vtjnash Aug 12, 2020

musm commented Aug 24, 2020

bnoordhuis commented Aug 24, 2020

musm commented Aug 30, 2020 •

edited

bnoordhuis commented Sep 1, 2020 •

edited

win,tty: win10 supports utf-16 surrogate pairs #2910

win,tty: win10 supports utf-16 surrogate pairs #2910

Conversation

bnoordhuis commented Jul 10, 2020

richardlau Jul 10, 2020

Choose a reason for hiding this comment

bnoordhuis Jul 10, 2020

Choose a reason for hiding this comment

vtjnash left a comment • edited

Choose a reason for hiding this comment

erw7 commented Jul 10, 2020

DHowett commented Jul 10, 2020 • edited

bnoordhuis commented Jul 11, 2020

bnoordhuis commented Jul 20, 2020

vtjnash Jul 20, 2020

Choose a reason for hiding this comment

musm Aug 4, 2020

Choose a reason for hiding this comment

vtjnash Aug 4, 2020

Choose a reason for hiding this comment

vtjnash Aug 12, 2020

Choose a reason for hiding this comment

musm commented Aug 24, 2020

bnoordhuis commented Aug 24, 2020

musm commented Aug 30, 2020 • edited

bnoordhuis commented Sep 1, 2020 • edited

vtjnash left a comment •

edited

DHowett commented Jul 10, 2020 •

edited

musm commented Aug 30, 2020 •

edited

bnoordhuis commented Sep 1, 2020 •

edited