Skip to content

Commit 990ec7f

Browse files
committed
Don’t implicitly cancel code completion requests on document edits
As the user types, we filter the code completion results. Cancelling the completion request on every keystroke means that we will never build the initial list of completion results for this code completion session if building that list takes longer than the user's typing cadence (eg. for global completions) and we will thus not show any completions.
1 parent 6c5f5c9 commit 990ec7f

File tree

2 files changed

+24
-18
lines changed

2 files changed

+24
-18
lines changed

Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,9 @@ package actor SourceKitLSPServer {
150150
}
151151
}
152152

153-
/// For all currently handled text document requests a mapping from the document to the corresponding request ID.
154-
private var inProgressTextDocumentRequests: [DocumentURI: Set<RequestID>] = [:]
153+
/// For all currently handled text document requests a mapping from the document to the corresponding request ID and
154+
/// the method of the request (ie. the value of `TextDocumentRequest.method`).
155+
private var inProgressTextDocumentRequests: [DocumentURI: [(id: RequestID, requestMethod: String)]] = [:]
155156

156157
var onExit: () -> Void
157158

@@ -550,18 +551,26 @@ package actor SourceKitLSPServer {
550551
// MARK: - MessageHandler
551552

552553
extension SourceKitLSPServer: QueueBasedMessageHandler {
554+
private enum ImplicitTextDocumentRequestCancellationReason {
555+
case documentChanged
556+
case documentClosed
557+
}
558+
553559
package nonisolated func didReceive(notification: some NotificationType) {
554560
let textDocumentUri: DocumentURI
561+
let cancellationReason: ImplicitTextDocumentRequestCancellationReason
555562
switch notification {
556563
case let params as DidChangeTextDocumentNotification:
557564
textDocumentUri = params.textDocument.uri
565+
cancellationReason = .documentChanged
558566
case let params as DidCloseTextDocumentNotification:
559567
textDocumentUri = params.textDocument.uri
568+
cancellationReason = .documentClosed
560569
default:
561570
return
562571
}
563572
textDocumentTrackingQueue.async(priority: .high) {
564-
await self.cancelTextDocumentRequests(for: textDocumentUri)
573+
await self.cancelTextDocumentRequests(for: textDocumentUri, reason: cancellationReason)
565574
}
566575
}
567576

@@ -582,11 +591,18 @@ extension SourceKitLSPServer: QueueBasedMessageHandler {
582591
///
583592
/// - Important: Should be invoked on `textDocumentTrackingQueue` to ensure that new text document requests are
584593
/// registered before a notification that triggers cancellation might come in.
585-
private func cancelTextDocumentRequests(for uri: DocumentURI) {
594+
private func cancelTextDocumentRequests(for uri: DocumentURI, reason: ImplicitTextDocumentRequestCancellationReason) {
586595
guard self.options.cancelTextDocumentRequestsOnEditAndCloseOrDefault else {
587596
return
588597
}
589-
for requestID in self.inProgressTextDocumentRequests[uri, default: []] {
598+
for (requestID, requestMethod) in self.inProgressTextDocumentRequests[uri, default: []] {
599+
if reason == .documentChanged && requestMethod == CompletionRequest.method {
600+
// As the user types, we filter the code completion results. Cancelling the completion request on every
601+
// keystroke means that we will never build the initial list of completion results for this code
602+
// completion session if building that list takes longer than the user's typing cadence (eg. for global
603+
// completions) and we will thus not show any completions.
604+
continue
605+
}
590606
logger.info("Implicitly cancelling request \(requestID)")
591607
self.messageHandlingHelper.cancelRequest(id: requestID)
592608
}
@@ -633,8 +649,8 @@ extension SourceKitLSPServer: QueueBasedMessageHandler {
633649

634650
/// - Important: Should be invoked on `textDocumentTrackingQueue` to ensure that new text document requests are
635651
/// registered before a notification that triggers cancellation might come in.
636-
private func registerInProgressTextDocumentRequest(_ request: any TextDocumentRequest, id: RequestID) {
637-
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].insert(id)
652+
private func registerInProgressTextDocumentRequest<T: TextDocumentRequest>(_ request: T, id: RequestID) {
653+
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].append((id: id, requestMethod: T.method))
638654
}
639655

640656
package func handle<Request: RequestType>(
@@ -644,7 +660,7 @@ extension SourceKitLSPServer: QueueBasedMessageHandler {
644660
) async {
645661
defer {
646662
if let request = params as? any TextDocumentRequest {
647-
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].remove(id)
663+
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].removeAll { $0.id == id }
648664
}
649665
}
650666

Sources/SwiftExtensions/AsyncQueue.swift

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,6 @@ package final class AsyncQueue<TaskMetadata: DependencyTracker>: Sendable {
8282

8383
package init() {}
8484

85-
package func cancelTasks(where filter: (TaskMetadata) -> Bool) {
86-
pendingTasks.withLock { pendingTasks in
87-
for task in pendingTasks {
88-
if filter(task.metadata) {
89-
task.task.cancel()
90-
}
91-
}
92-
}
93-
}
94-
9585
/// Schedule a new closure to be executed on the queue.
9686
///
9787
/// If this is a serial queue, all previously added tasks are guaranteed to

0 commit comments

Comments
 (0)