Skip to content

Commit 87af5b0

Browse files
authored
Merge pull request #1857 from ahoppen/no-implicit-completion-cancellation
2 parents 1a708ec + 990ec7f commit 87af5b0

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
@@ -144,8 +144,9 @@ package actor SourceKitLSPServer {
144144
}
145145
}
146146

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

150151
var onExit: () -> Void
151152

@@ -544,18 +545,26 @@ package actor SourceKitLSPServer {
544545
// MARK: - MessageHandler
545546

546547
extension SourceKitLSPServer: QueueBasedMessageHandler {
548+
private enum ImplicitTextDocumentRequestCancellationReason {
549+
case documentChanged
550+
case documentClosed
551+
}
552+
547553
package nonisolated func didReceive(notification: some NotificationType) {
548554
let textDocumentUri: DocumentURI
555+
let cancellationReason: ImplicitTextDocumentRequestCancellationReason
549556
switch notification {
550557
case let params as DidChangeTextDocumentNotification:
551558
textDocumentUri = params.textDocument.uri
559+
cancellationReason = .documentChanged
552560
case let params as DidCloseTextDocumentNotification:
553561
textDocumentUri = params.textDocument.uri
562+
cancellationReason = .documentClosed
554563
default:
555564
return
556565
}
557566
textDocumentTrackingQueue.async(priority: .high) {
558-
await self.cancelTextDocumentRequests(for: textDocumentUri)
567+
await self.cancelTextDocumentRequests(for: textDocumentUri, reason: cancellationReason)
559568
}
560569
}
561570

@@ -576,11 +585,18 @@ extension SourceKitLSPServer: QueueBasedMessageHandler {
576585
///
577586
/// - Important: Should be invoked on `textDocumentTrackingQueue` to ensure that new text document requests are
578587
/// registered before a notification that triggers cancellation might come in.
579-
private func cancelTextDocumentRequests(for uri: DocumentURI) {
588+
private func cancelTextDocumentRequests(for uri: DocumentURI, reason: ImplicitTextDocumentRequestCancellationReason) {
580589
guard self.options.cancelTextDocumentRequestsOnEditAndCloseOrDefault else {
581590
return
582591
}
583-
for requestID in self.inProgressTextDocumentRequests[uri, default: []] {
592+
for (requestID, requestMethod) in self.inProgressTextDocumentRequests[uri, default: []] {
593+
if reason == .documentChanged && requestMethod == CompletionRequest.method {
594+
// As the user types, we filter the code completion results. Cancelling the completion request on every
595+
// keystroke means that we will never build the initial list of completion results for this code
596+
// completion session if building that list takes longer than the user's typing cadence (eg. for global
597+
// completions) and we will thus not show any completions.
598+
continue
599+
}
584600
logger.info("Implicitly cancelling request \(requestID)")
585601
self.messageHandlingHelper.cancelRequest(id: requestID)
586602
}
@@ -627,8 +643,8 @@ extension SourceKitLSPServer: QueueBasedMessageHandler {
627643

628644
/// - Important: Should be invoked on `textDocumentTrackingQueue` to ensure that new text document requests are
629645
/// registered before a notification that triggers cancellation might come in.
630-
private func registerInProgressTextDocumentRequest(_ request: any TextDocumentRequest, id: RequestID) {
631-
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].insert(id)
646+
private func registerInProgressTextDocumentRequest<T: TextDocumentRequest>(_ request: T, id: RequestID) {
647+
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].append((id: id, requestMethod: T.method))
632648
}
633649

634650
package func handle<Request: RequestType>(
@@ -638,7 +654,7 @@ extension SourceKitLSPServer: QueueBasedMessageHandler {
638654
) async {
639655
defer {
640656
if let request = params as? any TextDocumentRequest {
641-
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].remove(id)
657+
self.inProgressTextDocumentRequests[request.textDocument.uri, default: []].removeAll { $0.id == id }
642658
}
643659
}
644660

Sources/SwiftExtensions/AsyncQueue.swift

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

8686
package init() {}
8787

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

0 commit comments

Comments
 (0)