Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16de20d
win,tty: win10 supports utf-16 surrogate pairs
bnoordhuis Jul 10, 2020
4e3ec66
fixup! win10 supports utf-16 surrogate pairs
bnoordhuis Jul 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/win/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ typedef struct {
uint32_t delayed_error;
} uv__ipc_socket_xfer_info_t;

extern int is_windows_10_or_greater;

int uv_tcp_listen(uv_tcp_t* handle, int backlog, uv_connection_cb cb);
int uv_tcp_accept(uv_tcp_t* server, uv_tcp_t* client);
int uv_tcp_read_start(uv_tcp_t* handle, uv_alloc_cb alloc_cb,
Expand Down
12 changes: 9 additions & 3 deletions src/win/tty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2123,9 +2123,9 @@ static int uv_tty_write_bufs(uv_tty_t* handle,
}

/* We wouldn't mind emitting utf-16 surrogate pairs. Too bad, the windows
* console doesn't really support UTF-16, so just emit the replacement
* character. */
if (utf8_codepoint > 0xffff) {
* 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;
}
Comment on lines +2126 to 2130
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump?


Expand Down Expand Up @@ -2155,6 +2155,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;
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/win/winapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
*/

#include <assert.h>
#include <string.h>

#include "uv.h"
#include "internal.h"

int is_windows_10_or_greater;

/* Ntdll function pointers */
sRtlGetVersion pRtlGetVersion;
Expand Down Expand Up @@ -51,6 +53,7 @@ void uv_winapi_init(void) {
HMODULE powrprof_module;
HMODULE user32_module;
HMODULE kernel32_module;
OSVERSIONINFOW info;

ntdll_module = GetModuleHandleA("ntdll.dll");
if (ntdll_module == NULL) {
Expand All @@ -59,6 +62,12 @@ void uv_winapi_init(void) {

pRtlGetVersion = (sRtlGetVersion) GetProcAddress(ntdll_module,
"RtlGetVersion");
if (pRtlGetVersion != NULL) {
memset(&info, 0, sizeof(info));
info.dwOSVersionInfoSize = sizeof(info);
pRtlGetVersion(&info);
is_windows_10_or_greater = (info.dwMajorVersion >= 10);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

pRtlNtStatusToDosError = (sRtlNtStatusToDosError) GetProcAddress(
ntdll_module,
Expand Down