-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refactored Activities for clarity and a consistent API #1993
base: main
Are you sure you want to change the base?
Refactored Activities for clarity and a consistent API #1993
Conversation
… UI. Improved aesthetics of notification banner
…he notification overlay UI previously just used for temporary notifications.
- Add system notification action button that, when clicked, focuses CodeEdit, runs the action, and dismisses the corresponding CodeEdit notification. - Dismissing a CodeEdit notification now also dismisses its corresponding system notification, and vice versa. - The notification panel in a workspace now closes when clicking outside of it, behaving like other menus. - Refactored notification state management: Moved display-related state from `NotificationService` to a dedicated view model to ensure notification panels remain independent across workspaces.
…ector can be enabled with a setting found in the hidden developer settings page.
…ification panel is presented. Removed notification tests and made the notification test in the dev inspector more configurable.
…nOverlay to notificationPanel. Fixed SwiftLint errors.
…distinction in name and have a consistent API when compared with Notifications.
![]() @thecoolwinter When testing, we are getting this error. Any idea what is going on here? |
That's a tough one I think I'd need to see more of the stack to determine what it is. I'll give it a go on my machine. |
The issue is that the updates are coming too fast and causing a crash in Swift's reference counting code in the Not entirely sure why this PR made this start crashing. Also not sure why this is being indexed for this test. May be worth making an issue for. Git Diff
diff --git a/CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument+Index.swift b/CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument+Index.swift
index 5a5a4620..0ffad45b 100644
--- a/CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument+Index.swift
+++ b/CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument+Index.swift
@@ -25,6 +25,10 @@ extension WorkspaceDocument.SearchState {
)
}
+ let (progressStream, continuation) = AsyncStream<Double>.makeStream()
+ // Dispatch this now, we want to continue after starting to monitor
+ Task { await self.monitorProgressStream(progressStream, activityId: activity.id) }
+
Task.detached {
let filePaths = self.getFileURLs(at: url)
let asyncController = SearchIndexer.AsyncManager(index: indexer)
@@ -33,16 +37,6 @@ extension WorkspaceDocument.SearchState {
// Batch our progress updates
var pendingProgress: Double?
- func updateProgress(_ progress: Double) async {
- await MainActor.run {
- self.indexStatus = .indexing(progress: progress)
- self.workspace.activityManager.update(
- id: activity.id,
- percentage: progress
- )
- }
- }
-
for await (file, index) in AsyncFileIterator(fileURLs: filePaths) {
_ = await asyncController.addText(files: [file], flushWhenComplete: false)
let progress = Double(index) / Double(filePaths.count)
@@ -54,7 +48,7 @@ extension WorkspaceDocument.SearchState {
// Only update UI every 100ms
if index == filePaths.count - 1 || pendingProgress != nil {
- await updateProgress(progress)
+ continuation.yield(progress)
pendingProgress = nil
}
}
@@ -77,6 +71,25 @@ extension WorkspaceDocument.SearchState {
}
}
+
+ /// Monitors a progress stream from ``addProjectToIndex()`` and updates ``indexStatus`` and the workspace's activity
+ /// manager accordingly.
+ ///
+ /// Without this, updates can come too fast for `Combine` to handle and can cause crashes.
+ ///
+ /// - Parameters:
+ /// - stream: The stream to monitor for progress updates, in %.
+ /// - activityId: The activity ID that's being monitored
+ @MainActor private func monitorProgressStream(_ stream: AsyncStream<Double>, activityId: String) async {
+ for await progressUpdate in stream.debounce(for: .milliseconds(10)) {
+ self.indexStatus = .indexing(progress: progressUpdate)
+ self.workspace.activityManager.update(
+ id: activityId,
+ percentage: progressUpdate
+ )
+ }
+ }
+
/// Retrieves an array of file URLs within the specified directory URL.
///
/// - Parameter url: The URL of the directory to search for files.
|
Description
This PR refactors the Activity Manager to provide a more Swift-like, type-safe API similar to our NotificationManager. It removes the dependency on NotificationCenter and provides a cleaner interface for managing activities within workspaces. This refactor aligns with CodeEdit’s architecture by ensuring activities are workspace-specific while keeping notifications global.
TL;DR:
TaskNotificationModel
→Activity
Note
To test this, go to Settings, press F12, toggle on "Show Internal Development Inspector". In a workspace, go to the Internal Development Inspector in the Inspector Area, and then test Activities using the Activities form.
Changes
Filesystem changes
New API
Activities can now be created, updated and deleted using a simple, fluent API:
Architecture Changes
ActivityManager
is now a properObservableObject
NotificationCenter
-based activity managementMigration
Updated existing activity creators to use new API:
CEActiveTask
WorkspaceDocument+Index
Checklist