-
Notifications
You must be signed in to change notification settings - Fork 317
Attach manifest to enable UTF-8 on Windows #3886
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
Conversation
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.
LGTM, thanks!
|
Would be nice ig it didnt drop support for anything before Malware 10 version 1903 |
|
It doesn't drop anything, as there was no support for Unicode (i.e. legacy -W API conversion) for Windows in libjxl tools to begin with. This at least adds UTF-8 for Windows 10 1903 and later. On earlier Windows versions it continues working just the same as today, i.e. ASCII only. |
|
Let me rephrase, maybe add something that works before 1903? |
There's no 'something' if you use Windows that is older than Windows 10 version 1903, other than patching the code to use -W API, but that would mean a complete overhaul of the entire codebase. Get-ChildItem *.png,*.jpg | ForEach-Object {$name = $_.BaseName;Rename-Item -Path $_ -NewName ($_ -replace $_.BaseName,'a');cjxl -e 9 -d 1 "a$($_.Extension)" "a.jxl";Rename-Item -Path "a$($_.Extension)" -NewName "$name$($_.Extension)";Rename-Item -Path 'a.jxl' -NewName "$name.jxl"} |
To put things into perspective: this addition was almost trivial, around 20 lines (of configuration, not even code), and covers >96% of Windows users. To cover the remaining <4% (and declining; also no longer supported by the OS vendor), the code changes required are non-trivial, need a more substantial developer effort and time, and can introduce new bugs that would need to be tested and addressed... |
(cherry picked from commit 7b70ef1)
(cherry picked from commit 7b70ef1)
He's not completely wrong, but like it's only an issue once you get to XP. The official release doesn't work on pre-7, but a build with clang can run just fine on XP, but it requires a change to the manifest. |
|
Nevermind, apparently this breaks UTF-8 support on 10. |
Description
Attach manifest to enable reading files with Unicode characters name (eg. Japanese, Russian, etc). Note that this only works for Windows 10 version 1903 and later. See this link for more information. This is taken from @scurest and @kmilos contribution, and uses libavif for its inspiration. Tested on MSVC with cl, clang-cl, clang-gnu frontend, and MSYS2 with ucrt64 (GCC toolchain) and clang64 (LLVM toolchain).
Fixes #683.
Pull Request Checklist
./ci.sh lintfor automatic code formatting.