Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Refactored Activities for clarity and a consistent API#1993

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
austincondiff wants to merge36 commits intoCodeEditApp:main
base:main
Choose a base branch
Loading
fromaustincondiff:activities-refactor

Conversation

@austincondiff
Copy link
Collaborator

@austincondiffaustincondiff commentedFeb 20, 2025
edited
Loading

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:

  • TaskNotificationModelActivity
  • Organized files
  • Activities API is now more consistent with Notifications API

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

CodeEdit/└── CodeEdit/│   └── Features/│       └── Activities/                               (previously ActivityViewer)│           ├── ActivityManager.swift                 (previously TaskNotificationHandler)│           ├── Models/│           │   └── CEActivity.swift                  (previously TaskNotificationModel)│           └── Views/│                ├── ActivityView.swift               (previously TaskNotificationView) │                └── ActivitiesDetailView.swift       (previously TaskNotificationsDetailView)└── CodeEditTests/    └── Features/        └── Activities/                               (previously ActivityViewer)            └── ActivityManagerTests.swift            (previously TaskNotificationHandlerTests)

New API

Activities can now be created, updated and deleted using a simple, fluent API:

@ObservedObjectvaractivityManager:ActivityManager// Createletactivity= activityManager.post(    title:"Indexing Files",    message:"Processing workspace files",    isLoading:true)// Create with priority (shows at top of list)letactivity= activityManager.post(    priority:true,    title:"Building Project",    message:"Compiling sources",    percentage:0.0,    isLoading:true)// UpdateactivityManager.update(    id: activity.id,    percentage:0.5,    message:"50% complete")// DeleteactivityManager.delete(id: activity.id)// Delete with delayactivityManager.delete(id: activity.id, delay:4.0)

Architecture Changes

  • Activities are now scoped to workspaces instead of being global
  • ActivityManager is now a properObservableObject
  • RemovedNotificationCenter-based activity management
  • Added proper type safety for activity parameters
  • Activities are now properly tracked and referenced by their own IDs

Migration

Updated existing activity creators to use new API:

  • Task execution activities inCEActiveTask
  • File indexing activities inWorkspaceDocument+Index
  • Updated tests to verify new API behavior

Checklist

  • I read and understood thecontributing guide as well as thecode of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

@austincondiff
Copy link
CollaboratorAuthor

image

@thecoolwinter When testing, we are getting this error. Any idea what is going on here?

@thecoolwinter
Copy link
Collaborator

thecoolwinter commentedFeb 28, 2025
edited
Loading

@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.

@thecoolwinter
Copy link
Collaborator

The issue is that the updates are coming too fast and causing a crash in Swift's reference counting code in theCombine framework. This patch fixes it by adding a debounce to the progress updates, and uses an async stream to pass the updates over async boundaries.

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.swiftindex 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.

… 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.
…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.
@austincondiff
Copy link
CollaboratorAuthor

austincondiff commentedMar 5, 2025
edited
Loading

@thecoolwinter that didn't seem to work. It builds fine but tests get hung up. Maybe it is because I merged with my changes from latest main. I've already spent an hour or two looking at this and I keep running into dead ends. Could you take another look when you get a chance?

@thecoolwinter
Copy link
Collaborator

Yeah absolutely. I'll lyk if I find anything

@austincondiffaustincondiff added the help wantedExtra attention is needed labelMar 13, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

@austincondiffaustincondiff

Labels

help wantedExtra attention is needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@austincondiff@thecoolwinter@tom-ludwig

[8]ページ先頭

©2009-2025 Movatter.jp