Skip to content

Added variable length Buffer.read* methods#93

Merged
dop251 merged 3 commits intodop251:masterfrom
stampy88:variable_length_reads
Feb 17, 2025
Merged

Added variable length Buffer.read* methods#93
dop251 merged 3 commits intodop251:masterfrom
stampy88:variable_length_reads

Conversation

@stampy88
Copy link
Copy Markdown
Contributor

@stampy88 stampy88 commented Feb 13, 2025 •
edited
Loading

Added variable length Buffer.read* methods

  • added readIntBE, readIntLE, readUIntBE, and readUIntLE.
  • also refactored out some common methods to create errors

Added variable length Buffer.read* methods
bd70b0b 
- added `readIntBE`, `readIntLE`, `readUIntBE`, and `readUIntLE`.
- also refactored out some common methods to create errors
dop251
dop251 requested changes Feb 14, 2025
View reviewed changes
Copy link
Copy Markdown
Owner

@dop251 dop251 left a comment

Choose a reason for hiding this comment

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

Thanks for submitting! I've added a couple of minor comments.

Comment thread buffer/buffer.go Outdated
}

func (b *Buffer) newArgumentOutOfRangeError(name string, v int64) *goja.Object {
return errors.NewError(b.r, nil, errors.ErrCodedOutOfRange, "The value of \"%s\" %d is out of range", name, v)
Copy link
Copy Markdown
Owner

@dop251 dop251 Feb 14, 2025

Choose a reason for hiding this comment

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

This should have RangeError as a prototype. You can either add errors.NewRangeError or just fetch the prototype here and pass it to NewError.

Comment thread buffer/buffer.go
proto.Set("readUInt32BE", b.readUInt32BE)
proto.Set("readUInt32LE", b.readUInt32LE)
proto.Set("readUIntBE", b.readUIntBE)
proto.Set("readUIntLE", b.readUIntLE)
Copy link
Copy Markdown
Owner

@dop251 dop251 Feb 14, 2025

Choose a reason for hiding this comment

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

It looks like, all ReadUInt* methods have ReadUint* aliases.

@stampy88
Copy link
Copy Markdown
Contributor Author

stampy88 commented Feb 17, 2025

@dop251, also wanted to ask you your thoughts about the best way to add the write* methods to Buffer. I don't see a good way to do it, so far.

@dop251
Copy link
Copy Markdown
Owner

dop251 commented Feb 17, 2025

What's the issue with the write* methods?

@dop251 dop251 merged commit ba90ff8 into dop251:master Feb 17, 2025
@stampy88
Copy link
Copy Markdown
Contributor Author

stampy88 commented Feb 17, 2025 •
edited
Loading

call.This.self is a *goja.typedArrayObject and I don't see a way to mutate the underlying *goja.uint8Array

@stampy88 stampy88 deleted the variable_length_reads branch February 17, 2025 18:34
@dop251
Copy link
Copy Markdown
Owner

dop251 commented Feb 17, 2025

Bytes(b.r, call.This) should give you access to the underlying []byte if I remember correctly, there should be no copying.

@stampy88
Copy link
Copy Markdown
Contributor Author

stampy88 commented Feb 17, 2025

OK, I'll give that a try, thanks!

benallfree pushed a commit to benallfree/goja_nodejs that referenced this pull request May 3, 2025
@stampy88 @benallfree
Added variable length Buffer.read* methods (dop251#93)
2804107 
* Added variable length `Buffer.read*` methods
- added `readIntBE`, `readIntLE`, `readUIntBE`, and `readUIntLE`.
- also refactored out some common methods to create errors

* added errors.NewRangeError. Also fixed mispelled `ErrCodedOutOfRange` -> `ErrCodeOutOfRange` (extra `d`)

* added the `readUint*` (small `i`) aliases

---------

Co-authored-by: dave sinclair <david_sinclair@cable.comcast.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dop251 dop251 dop251 requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stampy88 @dop251