Replace nwnsc subprocess with WASM compiler for diagnostics#81
Closed
PhilippeChab wants to merge 2 commits into
Closed
Replace nwnsc subprocess with WASM compiler for diagnostics#81PhilippeChab wants to merge 2 commits into
PhilippeChab wants to merge 2 commits into
Conversation
PR #62's intent was to migrate from spawning the native nwnsc binary to the in-process Beamdog nwn_script_comp; this commit accomplishes that goal but uses our WebAssembly build instead, which: - has multi-error recovery (one compile pass, every diagnostic in the file plus its #include'd helpers) - has no entry-point requirement (helper / include scripts can be edited and validated without a void main) - is ~210KB (vs 4-5MB native binary per platform) - works on every platform that runs Node, no separate Linux/Mac/ Windows binaries to ship DiagnosticsProvider rewritten: - Loads server/wasm/nwscript_compiler.{js,wasm} once on first use. - Per publish, creates a fresh compiler instance, sets collect-all-errors and require-entry-point=false, runs compile, drains the captured-error vector. - Resolver callback consults a per-call cache first then asks the workspace file system for any name the compiler requests, so nwscript.nss and #include'd files are picked up automatically. - Errors that fire inside an include are routed to the include's own URI (when we can resolve it via the workspace) rather than being attached to the file we were asked to check; the user sees the squiggle on the file that actually contains the bug. - Pre-seeds an empty diagnostic list for the target plus all transitive includes, so fixing an error clears the squiggle immediately. Removed: - spawn(child_process) machinery - getExecutablePath / hasSupportedOS - the WASM is platform-neutral - nwnsc command-line flag construction (-y -c -l -r -h -n -i) - stdout/stderr line-by-line parser (replaced by the collected-errors API) The compiler config keeps `enabled` and `verbose`; `os`, `nwnHome`, `nwnInstallation`, `reportWarnings` are now no-ops (kept in Config.ts for backwards-compat with any user settings.json that sets them). server/resources/compiler/{linux,mac,windows}/ binaries are no longer referenced; left in place so this commit can be reverted cleanly if the WASM approach hits a snag.
Owner
Author
|
Replaced by a clean PR from a branch off main. The original branch was based on top of an unrelated AST-tokenization commit, dragging that whole change into the diff. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #62's intent was to migrate from spawning the native
nwnscbinaryto the in-process Beamdog
nwn_script_comp. This PR accomplishes thatgoal but uses a WebAssembly build of the compiler instead, removing
the need for any native subprocess at all.
The WASM build comes from a fork of
neverwinter.nimextended withmulti-error recovery, no-entry-point validation, and a clean C API
suitable for Emscripten.
Why WASM over the native binary
its
#include'd helpers in a single compile, instead of stopping atthe first error.
void main. Helper / includescripts can be edited and validated as the user types.
runs. No separate Linux/Mac/Windows binaries to ship, no platform
detection, no
shell: true/powershell.exequirks.spawn/stdoutparsing, no per-keystrokeprocess startup overhead.
What changed
server/wasm/nwscript_compiler.{js,wasm}— replaced with the newWASM build (~210KB, was ~308KB).
server/src/Providers/DiagnosticsProvider.ts— rewritten:collect-all-errorsandrequire-entry-point=false, runs compile,drains the captured-error vector.
workspace file system for any name the compiler requests, so
nwscript.nssand#included files are picked up automatically.own URI when we can resolve it, so the squiggle lands on the file
that actually contains the bug.
transitive includes, so fixing an error clears the squiggle
immediately.
spawn(child_process)machinery,getExecutablePath/hasSupportedOS,nwnscflag construction(
-y -c -l -r -h -n -i), the line-by-line stdout parser.settings.json):compiler.os,compiler.nwnHome,compiler.nwnInstallation,compiler.reportWarnings.What's left in place for safety
server/resources/compiler/{linux,mac,windows}/nwnsc*— the nativebinaries are no longer referenced but kept on the branch so this
PR can be reverted cleanly if the WASM approach hits a snag.
Once the WASM approach is verified in the wild, those ~13MB of
binaries can be deleted in a follow-up.
Test plan
yarn compile:serverpasses (verified locally).yarn buildServerproduces a working bundle (verified locally)..nss, confirm squiggles appear.on three separate lines should show three squiggles in one pass.
void main)produces diagnostics directly, no longer requires the file to be
imported by a main script.
lib.nssreachedvia
#include "lib"frommain.nssshows onlib.nss.🤖 Generated with Claude Code