close
Skip to content

Fix node positional information for files with single-quotes in comments#164

Merged
shellscape merged 8 commits intoshellscape:masterfrom
nwalters512:fix/invalid-location-with-single-quote-comments
Oct 9, 2021
Merged

Fix node positional information for files with single-quotes in comments#164
shellscape merged 8 commits intoshellscape:masterfrom
nwalters512:fix/invalid-location-with-single-quote-comments

Conversation

@nwalters512
Copy link
Copy Markdown
Contributor

This resolves issue #163.

Please check one:

  • New tests created for this change
  • Tests updated for this change

This PR:

  • Adds new API
  • Extends existing API, backwards-compatible
  • Introduces a breaking change
  • Fixes a bug

This PR is my attempt at resolving the issue described in #163. It makes the new tests I added pass, and it doesn't break any existing tests. Unfortunately, most of the existing tests don't assert anything about positional information, so it's pretty difficult for me to assert that this doesn't adversely impact anything else. I'd be happy to augment some existing tests with positional information to help increase confidence.

The change to lib/nodes/inline-comment.js was necessary to get the following assertion about the end position of a comment to pass:

t.is(innerCommentNode.source.end.column, 6);

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 10, 2021

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@nwalters512 nwalters512 marked this pull request as ready for review August 10, 2021 22:58
@shellscape shellscape merged commit 4f36de8 into shellscape:master Oct 9, 2021
@shellscape
Copy link
Copy Markdown
Owner

thanks!

@nwalters512 nwalters512 deleted the fix/invalid-location-with-single-quote-comments branch November 3, 2021 17:21
@nwalters512
Copy link
Copy Markdown
Contributor Author

@shellscape do you have a rough estimate of when this will be released in a new version on npm?

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