Skip to content

update ui and add output #459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

lachieh
Copy link
Contributor

@lachieh lachieh commented Nov 5, 2024

No description provided.

@lachieh lachieh requested a review from rvolosatovs as a code owner November 5, 2024 23:53
Copy link
Member

@rvolosatovs rvolosatovs left a comment

Choose a reason for hiding this comment

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

A successful set should not produce any output, but once error handling is wired up we'd want to print the error if it occurred (i.e.

if (res != 0) {
alert("failed to set value")
return;
}
), if the byte is 1, then what follows is:

A little trick, however, is that for cases where LEB-128 encoded uint value is below 128, it is equal to a byte u8 repr of the value. (so for each i where i < 127, encoding is just i).
For now we could just read the whole buffer and if it's length is 128 or lower, skip the first byte and print the rest as a UTF-8 encoded string. You can see an example of that in decoding of set result

Just an idea for a potential follow-up (which I will try to work on myself anyway at some point)

LGTM, thanks!

@rvolosatovs rvolosatovs added this pull request to the merge queue Nov 6, 2024
@rvolosatovs
Copy link
Member

For reference, the string encoding is formally defined here: https://webassembly.github.io/spec/core/binary/values.html#names

Merged via the queue into bytecodealliance:main with commit 8eb346b Nov 6, 2024
36 checks passed
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