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

Add optional support for source locations #63

Merged
merged 5 commits into from
May 11, 2021

Conversation

devongovett
Copy link
Contributor

Fixes #44.

This adds a sourceLocations option, which is turned off by default, but when enabled, records source locations on each node in the loc property. These each have a start and end property, which include a line and column. Both lines and columns are 1-based, inclusive, which seems to match how most editors work.

Copy link
Member

@Scrum Scrum 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 your input - it's very cool. Could you please adjust a little PR

src/index.ts Outdated
};

let lastIndex = 0;
function getLoc(index: number) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it will be good if you take it into a separate method / file with explicit transmission of the required parameters.

I think it will be not bad if we use the name for example getLocation.

src/index.ts Outdated
const buf: NodeTag = {tag};

if (options.sourceLocations) {
buf.loc = {
Copy link
Member

Choose a reason for hiding this comment

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

I think it will be not bad if we use the name for example location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, if you prefer. loc is pretty commonly used e.g. in Babel and other parsers

@devongovett
Copy link
Contributor Author

Attempted to refactor into a separate file as requested. Let me know if you want other changes!

@Scrum
Copy link
Member

Scrum commented May 6, 2021

Attempted to refactor into a separate file as requested. Let me know if you want other changes!

Everything looks very cool. Can you add a description of this functionality to the documentation?

@devongovett
Copy link
Contributor Author

Done!

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.

[Feature] location property in AST
2 participants