Skip to content

Change endPosition to be the end of the node's length and add endPositionBeforeTrailingTrivia in its place #84

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 1 commit into from
Feb 13, 2019

Conversation

akyrtzi
Copy link
Contributor

@akyrtzi akyrtzi commented Feb 13, 2019

  • It is unintuitive that position points at the beginning of the node and endPosition points at the beginning of the trailing trivia. Pointing at the end of the node is more intuitive
  • Now there is a symmetry for position/endPosition and positionAfterSkippingLeadingTrivia/endPositionBeforeTrailingTrivia. position/endPosition are both "cost-free" while positionAfterSkippingLeadingTrivia/endPositionBeforeTrailingTrivia requires calculating the trivia length.

…ositionBeforeTrailingTrivia` in its place

* It is unintuitive that `position` points at the beginning of the node and `endPosition` points at the beginning of the trailing trivia. Pointing at the end of the node is more intuitive
* Now there is a symmetry for `position`/`endPosition` and `positionAfterSkippingLeadingTrivia`/`endPositionBeforeTrailingTrivia`. `position`/`endPosition` are both "cost-free" while `positionAfterSkippingLeadingTrivia`/`endPositionBeforeTrailingTrivia` requires calculating the trivia length.
@akyrtzi
Copy link
Contributor Author

akyrtzi commented Feb 13, 2019

@swift-ci Please test

Copy link
Contributor

@harlanhaskins harlanhaskins left a comment

Choose a reason for hiding this comment

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

Thanks!

@akyrtzi akyrtzi merged commit 6fca6e7 into swiftlang:master Feb 13, 2019
@akyrtzi akyrtzi deleted the endpositions branch February 13, 2019 19:10
adevress pushed a commit to adevress/swift-syntax that referenced this pull request Jan 14, 2024
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