Skip to content

Editorial: Add 'Position' and 'Original Position' records #194

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 3 commits into from
Apr 16, 2025

Conversation

szuend
Copy link
Collaborator

@szuend szuend commented Apr 7, 2025

This PR adds the 'Position' record consisting of a line plus column and a 'Original Position' record, consisting of a source index, line and column.

The mapping record thus changes to contain one generated position record and one original position record. This makes the intention of a mapping a bit more clear in the spec: e.g. a [[OriginalLine]] that is null while [[OriginalColumn]] is present, does not make a lot of sense (while possible to achieve in the encoding).

The PR also provides a general comparison function and replaces the adhoc "LessThan" mapping comparison in the index decoding algorithm.

We'll also use the 'Position' record in the spec text of the "scopes" proposal.

Preview: https://tc39.es/ecma426/pr/194/

@szuend szuend requested a review from nicolo-ribaudo April 7, 2025 06:37
Copy link
Collaborator

@takikawa takikawa left a comment

Choose a reason for hiding this comment

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

LGTM modulo some minor comments

@szuend
Copy link
Collaborator Author

szuend commented Apr 16, 2025

I rebased and made the change to "Original Position Record". Maybe have one last look if the rebase was done correctly before merging (fixed a warning about a trailing space as a drive-by). Thanks!

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Still looks good to me

@szuend szuend merged commit e20db3f into tc39:main Apr 16, 2025
3 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.

3 participants