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.

Sign up for GitHub

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

Jump to bottom

Use new API's from SwiftSyntax SourceEdit #1250

Merged
merged 3 commits into from
May 13, 2024
Merged

Use new API's from SwiftSyntax SourceEdit #1250

merged 3 commits into from
May 13, 2024

Conversation

kimdv
Copy link
Collaborator

@kimdv kimdv commented May 9, 2024

@kimdv kimdv self-assigned this May 9, 2024
@kimdv kimdv force-pushed the kimdv/2532-merge-incrementaledit-and-sourceedit branch 2 times, most recently from e0d540e to 60856ce Compare May 9, 2024 13:53
@kimdv
Copy link
Collaborator Author

kimdv commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test

@kimdv kimdv force-pushed the kimdv/2532-merge-incrementaledit-and-sourceedit branch from 7142cd5 to d520f98 Compare May 9, 2024 16:32
@kimdv
Copy link
Collaborator Author

kimdv commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test

@DougGregor
Copy link
Member

apple/swift-syntax#2604

@swift-ci please test Windows

@kimdv kimdv force-pushed the kimdv/2532-merge-incrementaledit-and-sourceedit branch 2 times, most recently from 040750f to 1fc05b9 Compare May 9, 2024 19:07
@kimdv
Copy link
Collaborator Author

kimdv commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test

@kimdv
Copy link
Collaborator Author

kimdv commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test windows

ahoppen
ahoppen reviewed May 9, 2024
View reviewed changes
Copy link
Collaborator

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks @kimdv. I think with your changes, the code can now be simplified even further.

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift Outdated Show resolved Hide resolved
Sources/SourceKitLSP/Swift/SwiftLanguageService.swift Show resolved Hide resolved
Sources/SourceKitLSP/Swift/SemanticTokens.swift Outdated Show resolved Hide resolved
Sources/SourceKitLSP/Swift/SemanticTokens.swift Outdated Show resolved Hide resolved
Sources/SourceKitLSP/Swift/DocumentFormatting.swift Outdated Show resolved Hide resolved
@kimdv kimdv force-pushed the kimdv/2532-merge-incrementaledit-and-sourceedit branch from 1fc05b9 to 9d7f767 Compare May 9, 2024 20:35
@kimdv kimdv requested a review from ahoppen May 9, 2024 20:35
@kimdv
Copy link
Collaborator Author

kimdv commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test

@ahoppen
Copy link
Collaborator

ahoppen commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test Windows

@kimdv kimdv force-pushed the kimdv/2532-merge-incrementaledit-and-sourceedit branch from 9d7f767 to d17ebf6 Compare May 9, 2024 22:23
@kimdv
Copy link
Collaborator Author

kimdv commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test

@ahoppen
Copy link
Collaborator

ahoppen commented May 9, 2024

apple/swift-syntax#2604

@swift-ci please test Windows

@kimdv kimdv force-pushed the kimdv/2532-merge-incrementaledit-and-sourceedit branch from d17ebf6 to 5ff6c33 Compare May 10, 2024 08:47
@kimdv kimdv mentioned this pull request May 10, 2024
Merged
@kimdv kimdv force-pushed the kimdv/2532-merge-incrementaledit-and-sourceedit branch from 5ff6c33 to ed61630 Compare May 10, 2024 08:51
@kimdv
Copy link
Collaborator Author

kimdv commented May 10, 2024

apple/swift-syntax#2604

@swift-ci please test

@kimdv
Copy link
Collaborator Author

kimdv commented May 10, 2024

apple/swift-syntax#2604

@swift-ci please test Windows

kimdv
kimdv commented May 10, 2024
View reviewed changes
Sources/SourceKitLSP/Swift/SemanticTokens.swift
@@ -58,7 +58,7 @@ extension SwiftLanguageService {
if let range = range.flatMap({ snapshot.byteSourceRange(of: $0) }) {
range
} else {
ByteSourceRange(offset: 0, length: await tree.totalLength.utf8Length)
await tree.range
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ahoppen we await here but also on line 65.
Would it not be best to change async let tree = syntaxTreeManager.syntaxTree(for: snapshot) -> let tree = await syntaxTreeManager.syntaxTree(for: snapshot)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The benefit of async let is allow us to compute the syntax tree and semantic highlighting tokens in parallel. If we await syntaxTreeManager.syntaxTree(for:), we would wait for the syntax tree to be computed before starting the computation of semantic tokens.

@ahoppen ahoppen merged commit 4bff560 into apple:main May 13, 2024
3 checks passed
@kimdv kimdv deleted the kimdv/2532-merge-incrementaledit-and-sourceedit branch May 17, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahoppen ahoppen ahoppen approved these changes

@benlangmuir benlangmuir Awaiting requested review from benlangmuir benlangmuir is a code owner

Assignees

@kimdv kimdv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kimdv @DougGregor @ahoppen