Skip to content

Support for Chinese Characters#14934

Merged
mergify[bot] merged 35 commits intodevelopfrom
wip/adr/utf8-friendly-getwindowsdocumentspath
Apr 16, 2026
Merged

Support for Chinese Characters#14934
mergify[bot] merged 35 commits intodevelopfrom
wip/adr/utf8-friendly-getwindowsdocumentspath

Conversation

@AdRiley
Copy link
Copy Markdown
Member

@AdRiley AdRiley commented Apr 2, 2026

Pull Request Description

Fixes an issue that if the project is in a folder path in Windows with a UTF-16 character, it fails to launch.

  • Uses PowerShell to get the folder path (rather than the registry).
  • Use W version of the Win32 API for controlling the working folder.
  • Use Win32 API for reading the command line arguments.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

@AdRiley AdRiley added CI: No changelog needed Do not require a changelog entry for this PR. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Apr 2, 2026
@AdRiley AdRiley force-pushed the wip/adr/utf8-friendly-getwindowsdocumentspath branch from 2afc141 to b4efa72 Compare April 3, 2026 09:54
Comment thread engine/runner/src/main/java/org/enso/runner/Main.java Outdated
*/
@CFunction
static native int PathFileExistsA(CCharPointer pszPath);
static native int PathFileExistsW(CShortPointer pszPath);
Copy link
Copy Markdown
Member

@JaroslavTulach JaroslavTulach Apr 8, 2026

Choose a reason for hiding this comment

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

  • replacing the API with PathFileExistsW might be the right direction
  • the fastest way to test these changes is sbt os-environment/test
    • builds small native image
    • runs tests in native image mode
    • way faster than building enso executable
  • there is a dedicated test
  • can it be enhanced with a sample demonstrating paths with non-ASCII characters?

Copy link
Copy Markdown
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

  • We should be using CTypeConversions, probably.
  • Great to have a test!

@Test
public void changeDirUnicode() throws IOException {
var tmpDir = TMP_DIR.newFolder().toPath();
var subDir = tmpDir.resolve("使用者");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice test!

return null;
}
return path.trim();
return asCharBuffer(buffer, length).toString();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe we should be using CTypeConversion, if possible:

Suggested change
return asCharBuffer(buffer, length).toString();
return CTypeConversion.toJavaString(buffer, length, StandardCharsets.UTF16_LE);

if that works and the test passes.

Comment thread engine/runner/src/main/java/org/enso/runner/Main.java Outdated
Comment thread engine/runner/src/main/java/org/enso/runner/Main.java Outdated
Comment thread engine/runner/src/main/java/org/enso/runner/Utils.java Outdated
@jdunkerley jdunkerley changed the title powershell version Support for Chinese Characters Apr 15, 2026
@jdunkerley jdunkerley marked this pull request as ready for review April 15, 2026 13:22
Comment thread app/project-manager-shim/src/projectService/ensoRunner.ts Outdated
Comment thread app/project-manager-shim/src/projectService/ensoRunner.ts Outdated
@jdunkerley jdunkerley force-pushed the wip/adr/utf8-friendly-getwindowsdocumentspath branch from ed1c920 to d089b47 Compare April 15, 2026 14:13
Comment thread app/project-manager-shim/src/desktopEnvironment.ts Outdated
Copy link
Copy Markdown
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

  • please refactor WindowsArguments to the sealed API pattern
  • consider malloc, but it is OK to keep the MAX_LENGTH if you want
  • otherwise I am glad we can handle Chines Characters now!

function getWindowsDocumentsPath() {
const out = childProcess.spawnSync(
'reg',
'powershell',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if we run on Windows that don't have powershell?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's been built into Windows since Windows 7 I think - so think ok to assume its there.

Comment thread engine/runner/src/main/java/org/enso/runner/Utils.java Outdated
Comment thread engine/runner/src/main/java/org/enso/runner/Utils.java Outdated
var res = PathFileExistsA(cPath.get());
try {
var buffer = stringAsWCharPtr(full);
var res = PathFileExistsW(buffer);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, let's use W version of the API methods.

@JaroslavTulach
Copy link
Copy Markdown
Member

Also please provide some description for this PR.

@jdunkerley jdunkerley force-pushed the wip/adr/utf8-friendly-getwindowsdocumentspath branch from 9220718 to 5752cab Compare April 16, 2026 09:15
@jdunkerley jdunkerley added the CI: Ready to merge This PR is eligible for automatic merge label Apr 16, 2026
@mergify mergify Bot merged commit 69056d0 into develop Apr 16, 2026
81 checks passed
@mergify mergify Bot deleted the wip/adr/utf8-friendly-getwindowsdocumentspath branch April 16, 2026 18:53
@JaroslavTulach
Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants