Skip to content

JavaScript: have File.setContents() take only the contents as a parameter#536

Merged
agarny merged 6 commits intoopencor:mainfrom
agarny:issue535
Feb 11, 2026
Merged

JavaScript: have File.setContents() take only the contents as a parameter#536
agarny merged 6 commits intoopencor:mainfrom
agarny:issue535

Conversation

@agarny
Copy link
Contributor

@agarny agarny commented Feb 11, 2026

Fixes #535.

Copilot AI review requested due to automatic review settings February 11, 2026 01:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the JavaScript bindings so File.setContents() accepts the file contents directly (e.g., a Uint8Array) rather than a (pointer, length) pair, aligning the JS API with the C++/Python ergonomics and simplifying test/demo code.

Changes:

  • Update the JS binding of File.setContents() to take a single JS value and convert it into bytes.
  • Refactor JavaScript binding tests and the browser demo to pass Uint8Array contents directly (removing explicit _malloc/_free usage in most places).
  • Update version-bumping scripts and bump VERSION.txt.

Reviewed changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/bindings/javascript/solver.secondorderrungekutta.test.js Switch to 1-arg setContents() and drop manual wasm memory management.
tests/bindings/javascript/solver.kinsol.test.js Same API update across KINSOL tests.
tests/bindings/javascript/solver.heun.test.js Same API update for Heun solver tests.
tests/bindings/javascript/solver.fourthorderrungekutta.test.js Same API update for RK4 tests.
tests/bindings/javascript/solver.forwardeuler.test.js Same API update for Forward Euler tests.
tests/bindings/javascript/solver.cvode.test.js Same API update across CVODE tests.
tests/bindings/javascript/solver.coverage.test.js Coverage tests updated to 1-arg setContents().
tests/bindings/javascript/sed.serialise.test.js Serialisation tests updated to 1-arg setContents().
tests/bindings/javascript/sed.instance.test.js Instance tests updated, but one call site still uses the old 2-arg form.
tests/bindings/javascript/sed.coverage.test.js SED coverage tests updated to 1-arg setContents().
tests/bindings/javascript/sed.basic.test.js Basic SED tests updated to 1-arg setContents().
tests/bindings/javascript/res/res/libopencor.js Demo now uses FileReader result and passes a Uint8Array to setContents().
tests/bindings/javascript/res/index.html Minor UI text spacing fix.
tests/bindings/javascript/logger.coverage.test.js Logger coverage tests updated to 1-arg setContents().
tests/bindings/javascript/file.type.test.js File type tests updated to 1-arg setContents().
tests/bindings/javascript/file.coverage.test.js Mostly updated, but still contains an old 2-arg setContents() call.
tests/bindings/javascript/file.child.test.js Helper refactored to pass contents directly instead of ptr/len.
tests/bindings/javascript/file.basic.test.js Basic file tests updated to 1-arg setContents().
src/bindings/javascript/file.cpp Changes the embind signature to accept a single JS value and convert to bytes.
newversion.bat Reworked version bumping to derive from VERSION.txt + date (needs quoting fixes).
newversion Same as above for *nix script (needs quoting fixes).
VERSION.txt Version bumped from 0.20260210.0 to 0.20260211.0.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@agarny agarny merged commit b9c357f into opencor:main Feb 11, 2026
30 checks passed
@agarny agarny deleted the issue535 branch February 11, 2026 05:05
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.

JavaScript: have File.setContents() take only the contents as a parameter

1 participant