Skip to content
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

Use both char and byte positions #458

Merged
merged 1 commit into from
May 12, 2023

Conversation

AntonyBlakey
Copy link
Contributor

No description provided.

@AntonyBlakey AntonyBlakey requested a review from a team as a code owner May 11, 2023 15:24
@AntonyBlakey AntonyBlakey enabled auto-merge May 11, 2023 15:24
@changeset-bot
Copy link

changeset-bot bot commented May 11, 2023

🦋 Changeset detected

Latest commit: f4eecf9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
changelog Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@AntonyBlakey AntonyBlakey requested review from OmarTawfik and removed request for a team May 11, 2023 15:25
@AntonyBlakey AntonyBlakey self-assigned this May 11, 2023
Copy link
Contributor

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

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

A couple of concerns about duplicate result in the getter API, and test snapshot offsets.

@AntonyBlakey AntonyBlakey force-pushed the AntonyBlakey/use-both-byte-and-char-positions branch from ca821a6 to f4eecf9 Compare May 12, 2023 07:43
@AntonyBlakey AntonyBlakey added this pull request to the merge queue May 12, 2023
Merged via the queue into main with commit c0fc7e9 May 12, 2023
@AntonyBlakey AntonyBlakey deleted the AntonyBlakey/use-both-byte-and-char-positions branch May 12, 2023 18:55
@github-actions github-actions bot mentioned this pull request May 12, 2023
OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this pull request May 12, 2023
Follow up on NomicFoundation#458

Turns out that using `#[napi(ts_return_type = "number")]` only affects the return type in the generated `index.d.ts` file, but not the actual return type at runtime.

Now we are using `usize`, which maps to JavaScript's `BigInt` by default, which is not ideal to work with at all. Using `u32` instead which maps to JavaScript's `number` type.

Also, adds a few missing `getter` attributes, and adds a test to check the new types.
OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this pull request May 12, 2023
Follow up on NomicFoundation#458

Turns out that using `#[napi(ts_return_type = "number")]` only affects the return type in the generated `index.d.ts` file, but not the actual return type at runtime.

Now we are using `usize`, which maps to JavaScript's `BigInt` by default, which is not ideal to work with at all. Using `u32` instead which maps to JavaScript's `number` type.

Also, adds a few missing `getter` attributes, and adds a test to check the new types.
OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this pull request May 12, 2023
Follow up on NomicFoundation#458

- make sure test snapshots padding uses chars instead of bytes
- fixed a minor bug with rendering error messages on empty source input, as it expects line/col, not start/end bytes.
OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this pull request May 12, 2023
Follow up on NomicFoundation#458

- make sure test snapshots padding uses chars instead of bytes
- fixed a minor bug with rendering error messages on empty source input, as it expects line/col, not start/end bytes.
OmarTawfik added a commit that referenced this pull request May 16, 2023
Turns out that using `#[napi(ts_return_type = "number")]` only affects
the return type in the generated `index.d.ts` file, but not the actual
return type at runtime. Currently we are using `usize`, which maps to
JavaScript's `BigInt` by default, which means our types are incorrect.

This PR uses `u32` instead which maps to JavaScript's `number` type.
Also adds a few missing `getter` attributes, and adds a test to check
the new types.

Follow up on #458
OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this pull request May 16, 2023
Follow up on NomicFoundation#458

- make sure test snapshots padding uses chars instead of bytes
- fixed a minor bug with rendering error messages on empty source input, as it expects line/col, not start/end bytes.
OmarTawfik pushed a commit that referenced this pull request May 16, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and publish to npm
yourself or [setup this action to publish
automatically](https://github.com/changesets/action#with-publishing). If
you're not ready to do a release yet, that's fine, whenever you add more
changesets to main, this PR will be updated.


# Releases
## @nomicfoundation/[email protected]

### Minor Changes

- [#458](#458)
[`c0fc7e9`](c0fc7e9)
Thanks [@AntonyBlakey](https://github.com/AntonyBlakey)! - Record both
character and byte offsets for input positions

- [#463](#463)
[`0958d6b`](0958d6b)
Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - use `number` and
getters in npm public API

## [email protected]

### Minor Changes

- [#458](#458)
[`c0fc7e9`](c0fc7e9)
Thanks [@AntonyBlakey](https://github.com/AntonyBlakey)! - Record both
character and byte offsets for input positions

- [#463](#463)
[`0958d6b`](0958d6b)
Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - use `number` and
getters in npm public API

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
OmarTawfik added a commit that referenced this pull request May 17, 2023
Follow up on #458

- make sure test snapshots padding uses chars instead of bytes
- fixed a minor bug with rendering error messages on empty source input, as it expects line/col, not start/end bytes.
OmarTawfik added a commit that referenced this pull request May 24, 2023
Follow up on #458

- make sure test snapshots padding uses chars instead of bytes
- fixed a minor bug with rendering error messages on empty source input,
as it expects line/col, not start/end bytes.
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