Merge wasm-playground branch into master#4
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request merges the wasm-playground branch into master, adding WebAssembly support to enable Hybroid compilation in a browser environment. This allows users to experiment with the Hybroid language through a web-based playground.
Changes:
- Added WebAssembly compilation support through a new
wasmpackage and WASM-specific entry point - Enhanced build script to support selective platform builds and added WASM as a build target
- Added project context documentation (GEMINI.md) for AI assistants
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| wasm/wasm.go | Implements WASM bindings for browser-based Hybroid compilation, exposing a hybroidCompile JavaScript function |
| main_wasm.go | WASM-specific entry point that registers the compile function and prevents program exit |
| main_native.go | Added build tag to exclude from WASM builds |
| utils/build_hybroid.py | Added selective build target support and WASM platform configuration |
| README.md | Updated build documentation to reflect optional platform parameter |
| GEMINI.md | Added comprehensive project context documentation for AI assistants |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| generator.ResetGlobalGeneratorValues() | ||
|
|
||
| gen.SetEnv(w.Env().Name, w.Env().Type) | ||
| gen.GenerateUsedLibaries(w.Env().UsedLibraries) |
There was a problem hiding this comment.
The method name 'GenerateUsedLibaries' contains a spelling error. It should be 'GenerateUsedLibraries' (with two 'r's). While this appears to be an existing issue in the codebase, this usage propagates the misspelling.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| if len(l.GetAlerts()) > 0 { | ||
| hasError := false | ||
| for _, a := range l.GetAlerts() { | ||
| if a.AlertType() == alerts.Error { | ||
| hasError = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| msg := formatAlerts(l.GetAlerts(), code) | ||
| if hasError { | ||
| return "", fmt.Errorf("%s", msg) | ||
| } | ||
| warnings.WriteString(msg) | ||
| } | ||
|
|
||
| p := parser.NewParser(tokensList) | ||
| program := p.Parse() | ||
|
|
||
| if len(p.GetAlerts()) > 0 { | ||
| hasError := false | ||
| for _, a := range p.GetAlerts() { | ||
| if a.AlertType() == alerts.Error { | ||
| hasError = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| msg := formatAlerts(p.GetAlerts(), code) | ||
| if hasError { | ||
| return "", fmt.Errorf("%s", msg) | ||
| } | ||
| warnings.WriteString(msg) | ||
| } | ||
|
|
||
| walker.SetupLibraryEnvironments() | ||
| w := walker.NewWalker("main.hyb", "main.lua") | ||
| w.SetProgram(program) | ||
|
|
||
| // Single file compilation, so no other walkers to share context with | ||
| walkers := make(map[string]*walker.Walker) | ||
| w.PreWalk(walkers) | ||
| w.Walk() | ||
| w.PostWalk() | ||
|
|
||
| if len(w.GetAlerts()) > 0 { | ||
| hasError := false | ||
| for _, a := range w.GetAlerts() { | ||
| if a.AlertType() == alerts.Error { | ||
| hasError = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| msg := formatAlerts(w.GetAlerts(), code) | ||
| if hasError { | ||
| return "", fmt.Errorf("%s", msg) | ||
| } | ||
| warnings.WriteString(msg) | ||
| } |
There was a problem hiding this comment.
The duplicate error-checking pattern (lines 75-89, 94-108, 120-134) should be extracted into a helper function to reduce code duplication and improve maintainability. This would make the compile function more concise and easier to maintain.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| # Clean previous builds only if building everything | ||
| if not target: | ||
| for file in os.listdir("build"): | ||
| os.remove("build/" + file) |
There was a problem hiding this comment.
The file removal logic will fail if the build directory contains subdirectories, as os.remove only works with files. Consider using os.path.isfile() to filter only files, or handle the exception when attempting to remove a directory.
| os.remove("build/" + file) | |
| path = os.path.join("build", file) | |
| if os.path.isfile(path): | |
| os.remove(path) |
| return "expected 1 argument" | ||
| } | ||
| if args[0].Type() != js.TypeString { | ||
| return "expected string" |
There was a problem hiding this comment.
The error messages returned to JavaScript callers are very generic ('expected 1 argument', 'expected string'). Consider making these more descriptive, such as 'hybroidCompile expects exactly 1 argument (code as string)' to provide better developer experience for JavaScript users of the API.
| return "expected 1 argument" | |
| } | |
| if args[0].Type() != js.TypeString { | |
| return "expected string" | |
| return "hybroidCompile expects exactly 1 argument (code as string)" | |
| } | |
| if args[0].Type() != js.TypeString { | |
| return "hybroidCompile expects its first argument to be a string containing the code to compile" |
| } | ||
| return sb.String() | ||
| } | ||
|
|
There was a problem hiding this comment.
The compile function lacks a documentation comment. As the core functionality of this package, it should have a comment explaining its purpose, parameters, and return values. Consider adding: '// compile transpiles Hybroid source code to Lua, collecting and formatting any warnings or errors encountered during lexing, parsing, and semantic analysis.'
| // compile transpiles Hybroid source code to Lua, collecting and formatting any | |
| // warnings or errors encountered during lexing, parsing, and semantic analysis. | |
| // The input parameter code is the Hybroid source as a string. It returns the | |
| // generated Lua source code, or a non-nil error if a fatal issue is detected. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@Tasty-Kiwi I've opened a new pull request, #5, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@Tasty-Kiwi I've opened a new pull request, #6, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Fix spelling: GenerateUsedLibaries -> GenerateUsedLibraries Co-authored-by: Tasty-Kiwi <26067386+Tasty-Kiwi@users.noreply.github.com> * Restore test directory files that were accidentally modified Co-authored-by: Tasty-Kiwi <26067386+Tasty-Kiwi@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Tasty-Kiwi <26067386+Tasty-Kiwi@users.noreply.github.com>
* Initial plan * Extract duplicate error-checking pattern into processAlerts helper Co-authored-by: Tasty-Kiwi <26067386+Tasty-Kiwi@users.noreply.github.com> * Remove unnecessary format string in processAlerts Co-authored-by: Tasty-Kiwi <26067386+Tasty-Kiwi@users.noreply.github.com> * Use errors.New instead of fmt.Errorf for pre-formatted strings Co-authored-by: Tasty-Kiwi <26067386+Tasty-Kiwi@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Tasty-Kiwi <26067386+Tasty-Kiwi@users.noreply.github.com>
No description provided.