-
Notifications
You must be signed in to change notification settings - Fork 5
Merge in upstream binary-parser #18
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
Conversation
Was "assert" called "validate" in an older version? Can't find any use or definition of "validate" in the current one.
Refactor CircleCI config
Publish from CircleCI
…nto Simran-B-patch-1 # Conflicts: # example/jpg.js
JPG example should use assert, not validate keichi#80
Inject Buffer into the vm context
Fix module not found error when running the examples
Statically declare all parser methods
Refactoring to leverage TS language features
add saveoffset method
change saveoffset -> saveOffset
Run tests in the browser
Rename skip() to seek()
Typescript support fixes.
Don't need to use npx in npm-scripts
There are still a couple of outstanding bugs in the tests from my resolution, so I'll work on resolving those |
All the tests are now passing! Sorry for the noise getting to this point - I hadn't noticed the tests weren't running at first. |
Thanks for this great job. I know that the PullRequest for BigInt support is now merged in the smart-buffer package, so there is no more reason to not resync with upstream binary-parser. However, it will require to add additional tests (at least for BigInt encoding) and I will have to switch my mind to move to TypeScript. I will requires some delay as I've switched to another legacy C++ project. So don't blame me if it takes sometime for your PR to be merged. B.R. |
Awesome! Thanks for the feedback. No worries about this taking time. I was using this for a week long project and wanted to make available the changes I had made. I don't expect this to get merged any time soon. Likewise it may be a while before I find the time to add tests but I will get around to doing it in the next few weeks. Thanks again! |
It looks like the upstream has made a bunch of changes since your original (awesome) PR was filed.
I happened to need some of the additional features (particularly Int64s) so did my best to resolve the conflicts.
Much of this was manual so I have undoubtedly made mistakes, but I hope it's at least helpful!
If you spot any bugs I should fix, let me know. Alternatively if there are other change you want to make, the PR is filed with "allow edits from maintainers enabled".
The main remaining TODOs are commented in the code - additional parsers were added upstream,
. pointer
and.saveOffset
(possibly internal? definitely undocumented). These still need encoders to be implemented, but I haven't done that yet as I'm not sure how.Hopefully this PR is helpful!