Virtualize path and file system operations #357
Conversation
|
Nice! But please remove all useless whitespace changes from the diff. |
There was a problem hiding this comment.
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.
Compare with the previous output:
While the Compiling: and /!!!\ Error messages are not synchronized (would be easy to fix), each error is properly reported separately and not interleaved.
Co-authored-by: Lionel Parreaux <[email protected]>
Co-authored-by: Lionel Parreaux <[email protected]>
Co-authored-by: Lionel Parreaux <[email protected]>
LPTK
left a comment
LPTK
left a comment
There was a problem hiding this comment.
Ok, it mostly LGTM, but please address these extra commnets and what we discussed by messages.
| val preludeFile = preludePath | ||
| val runtimeFile = rootPath/"hkmc2"/"shared"/"src"/"test"/"mlscript-compile"/"Runtime.mjs" | ||
| val termFile = rootPath/"hkmc2"/"shared"/"src"/"test"/"mlscript-compile"/"Term.mjs", | ||
| mkRaise = ReportFormatter(System.out.println, colorize = false).mkRaise |
There was a problem hiding this comment.
Why no color there?
| mkRaise = ReportFormatter(System.out.println, colorize = false).mkRaise | |
| mkRaise = ReportFormatter(System.out.println, colorize = true).mkRaise |
| import _root_.io.methvin.better.files.* | ||
| import _root_.io.methvin.watcher.{DirectoryWatcher, PathUtils} | ||
| import _root_.io.methvin.watcher.{DirectoryChangeEvent, DirectoryChangeListener} | ||
| import _root_.io.methvin.watcher.hashing.{FileHash, FileHasher} |
There was a problem hiding this comment.
| import _root_.io.methvin.better.files.* | |
| import _root_.io.methvin.watcher.{DirectoryWatcher, PathUtils} | |
| import _root_.io.methvin.watcher.{DirectoryChangeEvent, DirectoryChangeListener} | |
| import _root_.io.methvin.watcher.hashing.{FileHash, FileHasher} | |
| import _root_.io.methvin | |
| import methvin.better.files.* | |
| import methvin.watcher.{DirectoryWatcher, PathUtils, DirectoryChangeEvent, DirectoryChangeListener} | |
| import methvin.watcher.hashing.{FileHash, FileHasher} |
| /** Check if the term satisfies the predicate. The reason I did not use | ||
| * `subTerms` and `subStatements` for traversal is that they consume a | ||
| * considerable amount of time and memory, and they fail to terminate on | ||
| * `Rule.mls`. */ |
There was a problem hiding this comment.
This description sounds like the problem isn't purely time and memory, but more problematically these things having the wrong complexity. It's easy to imagine, given that they use list concatenation. Could you try making them return Vectors instead?
There was a problem hiding this comment.
And if that still doesn't work well enough, it'll be better to have aforeach method that doesn't recurse deeply into the term (recursion should be done by the caller, for finer control).
2 participants
When I make web demos, one of the headaches is that
os.Pathdoesn’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 a minimal web demo to showcase the ability to compile multiple files.This will be done in a separate PR.