-
Notifications
You must be signed in to change notification settings - Fork 254
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
Add a work done progress that shows the index progress #1275
Conversation
for file in files { | ||
if case .scheduled(let task) = self.indexStatus[file] { | ||
self.indexStatus[file] = .executing(task) | ||
} | ||
} | ||
case .cancelledToBeRescheduled: | ||
for file in files { | ||
if case .executing(let task) = self.indexStatus[file] { | ||
self.indexStatus[file] = .scheduled(task) | ||
} | ||
} | ||
case .finished: | ||
for file in files { | ||
self.indexStatus[file] = .upToDate | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we log an error or something if a file is in an unexpected state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea 👍🏽
} | ||
|
||
func startProgressImpl(server: SourceKitLSPServer) async { | ||
await server.waitUntilInitialized() | ||
activeTasks += 1 | ||
guard await server.capabilityRegistry?.clientCapabilities.window?.workDoneProgress ?? false else { | ||
// If the client doesn't support workDoneProgress, keep track of the active task count but don't update the state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment talks about handling the case where this is called before initialization finishes, I guess that's no longer relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well spotted 👍🏽
@swift-ci Please test |
@swift-ci Please test |
@swift-ci Please test Windows |
@swift-ci Please test |
@swift-ci Please test Windows |
1 similar comment
@swift-ci Please test Windows |
@swift-ci Please test |
This simplifies the implementation.
This fixes a bug where `indexTaskDidFinish` would also get called when a task is cancelled to be rescheduled.
…lback This follows the general paradigm that callbacks shouldn’t carry much state and instead only notify an observer that state has changed, which the observer can then poll.
…an be executed before SourceKit-LSP is initialized
…o be executed in any order
This makes it a lot easier to work on background indexing because you can easily see how background indexing is making progress. Resolves apple#1257 rdar://127474057
@swift-ci Please test |
@swift-ci Please test Windows |
1 similar comment
@swift-ci Please test Windows |
reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void, | ||
indexTasksWereScheduled: @Sendable @escaping (Int) -> Void, | ||
indexTaskDidFinish: @Sendable @escaping () -> Void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might need a ServerCallbacks
struct soon 😬
This makes it a lot easier to work on background indexing because you can easily see how background indexing is making progress.
Resolves #1257
rdar://127474057