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

Support completionItem/resolve to compute documentation of a code completion item #1938

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jan 21, 2025

Fixes #1935

@ahoppen ahoppen changed the title Support completionItem/resolve to resolve compute documentation of a code completion item Support completionItem/resolve to compute documentation of a code completion item Jan 27, 2025
@ahoppen ahoppen force-pushed the completion-item-resolve branch from bf7c123 to dcf2b92 Compare January 27, 2025 16:44
@ahoppen
Copy link
Member Author

ahoppen commented Jan 27, 2025

@swift-ci Please test

Comment on lines 103 to 104
case is CompletionItemResolveRequest:
self = .freestanding
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a request, not a notification, right? Also shouldn't it be considered a document request?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, good catch. Also just noticed that we do have it in the initializers for requests already.

Also shouldn't it be considered a document request?

I don’t think so. It’s not necessary to process any pending edits to resolve a code completion item. Either the session is still valid, in which can we can resolve independently of document edits or not, in which case we will error in either case.

@@ -134,7 +158,6 @@ class CodeCompletionSession {
session.snapshot.uri == snapshot.uri
&& session.position == completionPosition
&& session.compileCommand == compileCommand
&& session.clientSupportsSnippets == clientSupportsSnippets
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to check whether the capabilities match, or does that not matter?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don’t think that we need to. The only case where this would change is if the client initiated a code completion session and then dynamically registered the snippet support capability, which would be very unlikely since any of that dynamic registration would happen all up-front. And if should get it wrong, it’s not going to be a fatal error, the results might just look a little off (eg. no snippets even though client supports them).

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…pletion item

Fixes swiftlang#1935
@ahoppen ahoppen force-pushed the completion-item-resolve branch from dcf2b92 to f6b83db Compare February 5, 2025 18:26
@ahoppen
Copy link
Member Author

ahoppen commented Feb 5, 2025

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge February 5, 2025 18:27
@ahoppen
Copy link
Member Author

ahoppen commented Feb 6, 2025

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 1f17adb into swiftlang:main Feb 6, 2025
3 checks passed
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.

Implement completionItem/resolve to retrieve a completion item's documentation
2 participants