Skip to content

[SwiftSyntax] Simplify AbsolutePosition offset calculation and support columns #16131

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

Conversation

harlanhaskins
Copy link
Contributor

This patch simplifies the calculation and structure of AbsolutePosition, in preparation for DiagnosticEngine changes to easily create SourceLocations.

@harlanhaskins harlanhaskins requested a review from nkcsgexi April 24, 2018 15:45
/// Specifically, walks through the trivia and keeps track of every newline
/// to give a number of how many newlines and UTF8 characters appear in the
/// trivia, along with the UTF8 offset of the last column.
func characterSizes() -> (lines: Int, lastColumn: Int, utf8Length: Int) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Surprisingly, this code was never called.

@harlanhaskins
Copy link
Contributor Author

@nkcsgexi I'm wondering if we even need the utf-16 encoding.

@nkcsgexi
Copy link
Contributor

nkcsgexi commented Apr 24, 2018

@harlanhaskins if we have the character offset, then we don't need utf16-based offset i presume. The idea is utf16-based offset is easier to bridge to character offset.

Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

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

LGTM!

@harlanhaskins
Copy link
Contributor Author

The calculation is a little bit wrong, I'm investigating now.

}

internal func add(columns: Int) {
print("adding \(columns) column bytes")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove before merging

@harlanhaskins
Copy link
Contributor Author

Wow, very simple bug.

@swift-ci please smoke test

@harlanhaskins harlanhaskins changed the title [SwiftSyntax] [WIP] Simplify AbsolutePosition offset calculation and support columns [SwiftSyntax] Simplify AbsolutePosition offset calculation and support columns Apr 24, 2018
@nkcsgexi nkcsgexi merged commit d46ebe5 into swiftlang:master Apr 24, 2018
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