Skip to content

Conversation

@chengluyu
Copy link
Member

@chengluyu chengluyu commented Dec 1, 2025

When I make web demos, one of the headaches is that os.Path doesn’t compile when the target is JavaScript. I made a hack on my web demo branch. But the implementation is a bit crappy, especially after merging the latest changes for a few times. Therefore, I want to introduce a path representation and abstract read operations that both supports a real file system or a virtual one (e.g., when the compiler is running on the web).

This PR intends to do the following things.

  • Provide an abstraction layer of path and file system operations.
  • Call the MLscript compiler from JavaScript.
    • Compile single MLscript source file.
    • Compile multiple MLscript source files.
  • Provide a minimal web demo to showcase the ability to compile multiple files. This will be done in a separate PR.

@chengluyu chengluyu marked this pull request as ready for review December 11, 2025 10:46
@LPTK
Copy link
Contributor

LPTK commented Dec 11, 2025

Nice! But please remove all useless whitespace changes from the diff.

@chengluyu chengluyu requested a review from LPTK December 11, 2025 18:32
Copy link
Contributor

@LPTK LPTK left a comment

Choose a reason for hiding this comment

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

Nice work!

However, your changes broke diagnostic reporting.

First, the diagnostics are no longer colored in the console.

Second, they are no longer synchronized. For instance, add a bunch of errors in compilation-test files and watch the output get mangled.

Screenshot 2025-12-12 at 15 00 48

Compare with the previous output:

Screenshot 2025-12-12 at 15 02 47

While the Compiling: and /!!!\ Error messages are not synchronized (would be easy to fix), each error is properly reported separately and not interleaved.

Copy link
Contributor

@LPTK LPTK left a comment

Choose a reason for hiding this comment

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

Ok, it mostly LGTM, but please address these extra commnets and what we discussed by messages.

chengluyu and others added 2 commits December 13, 2025 15:04
Co-authored-by: Lionel Parreaux <[email protected]>
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.

2 participants