Skip to content

Support printing unicode characters on windows #449

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

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

andrjohns
Copy link
Contributor

From #447, Unicode characters are not correctly printed to the console with console.log in some cases

@andrjohns
Copy link
Contributor Author

@ahaoboy Can you try this branch to check that it works for you? All works as expected for me locally, but good to double-check

@ahaoboy
Copy link

ahaoboy commented Jun 28, 2024

Can you try this branch to check that it works for you?

Cool~ It works!

@chqrlie
Copy link
Collaborator

chqrlie commented Jun 28, 2024

We might want to add an alternative function JS_ToWStringLen to convert a JS_STRING to a UTF-16 encoded wide string and use that for output. It would help if Microsoft would finally make it easy to use UTF-8 like the rest of the world.

@saghul
Copy link
Contributor

saghul commented Aug 1, 2024

@chqrlie Do you have any objections?

quickjs-libc.c Outdated
}
MultiByteToWideChar(CP_UTF8, 0, str, len, wstr, wlen);
wstr[wlen] = L'\0';
WriteConsoleW(hConsole, wstr, wlen, &written, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this work?

DWORD prev = GetConsoleOutputCP();
SetConsoleOutputCP(CP_UTF8);
WriteConsoleA(hConsole, str, len, &written, NULL);
SetConsoleOutputCP(prev);

Maybe you don't even need to use WriteConsoleA:

DWORD prev = GetConsoleOutputCP();
SetConsoleOutputCP(CP_UTF8);
fwrite(str, 1, len, con);
SetConsoleOutputCP(prev);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bnoordhuis Apologies for the delay, WriteConsoleA still appeared to be needed, but prints successfully without needing the wstr conversion - so changed

quickjs-libc.c Outdated
@@ -3882,17 +3882,41 @@ static JSValue js_print(JSContext *ctx, JSValue this_val,
const char *str;
size_t len;

#ifdef _WIN32
Copy link
Contributor

Choose a reason for hiding this comment

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

There are barely a couple of lines of shared code here. Perhaps defining the whole function once for Unix one for Windows would be more readable here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! Separated into two definitions, ready for another look

@chqrlie chqrlie dismissed their stale review September 28, 2024 23:02

obsolete

@saghul saghul merged commit 72d4587 into quickjs-ng:master Sep 30, 2024
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants