diff --git a/Sources/Lithe/Models/AppModel/AppModel.swift b/Sources/Lithe/Models/AppModel/AppModel.swift index c4eff7bb3..8ea31ae71 100644 --- a/Sources/Lithe/Models/AppModel/AppModel.swift +++ b/Sources/Lithe/Models/AppModel/AppModel.swift @@ -1136,8 +1136,8 @@ final class AppModel: ObservableObject, Identifiable { workspaceFeature.cancelProjectItemDeletion() } - func confirmProjectItemDeletion() async { - await workspaceFeature.confirmProjectItemDeletion() + func confirmProjectItemDeletion(_ request: ProjectItemDeletionRequest) async { + await workspaceFeature.confirmProjectItemDeletion(request) } func revealProjectItemInFinder(_ url: URL) { diff --git a/Sources/Lithe/Views/Workspace/ProjectSidebarView.swift b/Sources/Lithe/Views/Workspace/ProjectSidebarView.swift index b6c09af57..20d8fe0f3 100644 --- a/Sources/Lithe/Views/Workspace/ProjectSidebarView.swift +++ b/Sources/Lithe/Views/Workspace/ProjectSidebarView.swift @@ -105,17 +105,18 @@ struct ProjectSidebarView: View { get: { model.pendingProjectItemDeletion != nil }, set: { if !$0 { model.cancelProjectItemDeletion() } } ), - titleVisibility: .visible - ) { + titleVisibility: .visible, + presenting: model.pendingProjectItemDeletion + ) { request in Button("Move to Trash", role: .destructive) { - Task { await model.confirmProjectItemDeletion() } + Task { await model.confirmProjectItemDeletion(request) } } .lithePointer() Button("Cancel", role: .cancel) { model.cancelProjectItemDeletion() } .lithePointer() - } message: { + } message: { _ in Text("The item can be recovered from the macOS Trash.") } } diff --git a/Sources/LitheWorkspaceModule/Application/WorkspaceFeatureModel.swift b/Sources/LitheWorkspaceModule/Application/WorkspaceFeatureModel.swift index fe1f17b9a..c66d171de 100644 --- a/Sources/LitheWorkspaceModule/Application/WorkspaceFeatureModel.swift +++ b/Sources/LitheWorkspaceModule/Application/WorkspaceFeatureModel.swift @@ -528,11 +528,19 @@ package final class WorkspaceFeatureModel: ObservableObject { pendingProjectItemDeletion = nil } - package func confirmProjectItemDeletion() async { - guard let request = pendingProjectItemDeletion else { return } - pendingProjectItemDeletion = nil + package func confirmProjectItemDeletion(_ request: ProjectItemDeletionRequest) async { + if pendingProjectItemDeletion?.id == request.id { + pendingProjectItemDeletion = nil + } + guard !isPerformingProjectItemOperation, + isWorkspaceURL(request.url), + request.url.standardizedFileURL != workspaceURL?.standardizedFileURL else { return } isPerformingProjectItemOperation = true - await recordHistory?(request.url, .beforeDelete) + // Update the visible tree before waiting for the native Trash operation. + // A failed operation reloads the disk snapshot below to restore the item. + removeProjectItemFromSnapshot(request.url) + // The system Trash is the recovery boundary. Recording every descendant + // first would make deleting a directory scale with its entire file tree. let fileOperations = self.fileOperations let errorMessage = await Task.detached(priority: .userInitiated) { () -> String? in do { @@ -545,6 +553,7 @@ package final class WorkspaceFeatureModel: ObservableObject { isPerformingProjectItemOperation = false if let errorMessage { notify?(errorMessage) + await refreshCurrent() return } closeDocuments?(request.url) @@ -792,6 +801,23 @@ package final class WorkspaceFeatureModel: ObservableObject { return childPath == parentPath || childPath.hasPrefix(parentPath + "/") } + private func removeProjectItemFromSnapshot(_ targetURL: URL) { + projectFiles.removeAll { urlContains(targetURL, child: $0) } + rootNode = rootNode.flatMap { removingProjectItem(targetURL, from: $0) } + } + + private func removingProjectItem(_ targetURL: URL, from node: FileNode) -> FileNode? { + guard node.url.standardizedFileURL != targetURL.standardizedFileURL else { return nil } + guard let children = node.children else { return node } + return FileNode( + url: node.url, + isDirectory: node.isDirectory, + children: children.compactMap { removingProjectItem(targetURL, from: $0) }, + collapsedAncestorPaths: node.collapsedAncestorPaths, + isInsideSourceRoot: node.isInsideSourceRoot + ) + } + private func availableDuplicateURL(for sourceURL: URL) -> URL { let parent = sourceURL.deletingLastPathComponent() let fileExtension = sourceURL.pathExtension diff --git a/Tests/LitheTests/LitheCoreLogicTests.swift b/Tests/LitheTests/LitheCoreLogicTests.swift index 7aef674b9..1ed1c9b80 100644 --- a/Tests/LitheTests/LitheCoreLogicTests.swift +++ b/Tests/LitheTests/LitheCoreLogicTests.swift @@ -2958,6 +2958,83 @@ struct EditorDocumentTests { #expect(gitRefreshCount == 1) } + @Test + @MainActor + func capturedProjectDeletionSurvivesConfirmationDialogDismissal() async throws { + let workspace = URL(fileURLWithPath: "/tmp/lithe-delete-confirmation") + let target = workspace.appendingPathComponent("obsolete.swift") + let fileOperations = RecordingTrashWorkspaceFileOperations() + let operations = SequencedWorkspaceOperations( + snapshotAvailability: [true, false], + files: [target] + ) + var historyRecordCount = 0 + let model = makeWorkspaceObservationUnitModel( + operations: operations, + fileOperations: fileOperations, + provider: SequencedGitWatchContextProvider([nil]), + watcherFactory: TestDirectoryWatcherFactory(), + refreshGit: {}, + recordHistory: { _, _ in historyRecordCount += 1 } + ) + model.beginWorkspace(at: workspace, visibilityRules: .default) + _ = await model.rebuild(at: workspace, rules: .default, isCurrent: { true }) + model.requestDeleteProjectItem(at: target, isDirectory: false) + let request = try #require(model.pendingProjectItemDeletion) + + // SwiftUI dismisses the confirmation dialog before its asynchronous + // action runs, so the captured request must not depend on pending state. + model.cancelProjectItemDeletion() + let deletionTask = Task { await model.confirmProjectItemDeletion(request) } + + for _ in 0..<100 where !fileOperations.hasStarted { + try? await Task.sleep(for: .milliseconds(10)) + } + + #expect(fileOperations.hasStarted) + #expect(model.projectFiles.isEmpty) + #expect(model.rootNode?.children?.isEmpty == true) + + fileOperations.release() + await deletionTask.value + + #expect(model.pendingProjectItemDeletion == nil) + #expect(fileOperations.trashedURLs == [target.standardizedFileURL]) + #expect(historyRecordCount == 0) + #expect(model.projectFiles.isEmpty) + #expect(model.rootNode?.children?.isEmpty == true) + } + + @Test + @MainActor + func failedProjectDeletionRestoresOptimisticallyRemovedItem() async { + let workspace = URL(fileURLWithPath: "/tmp/lithe-delete-failure") + let target = workspace.appendingPathComponent("still-here.swift") + let operations = SequencedWorkspaceOperations( + snapshotAvailability: [true, true], + files: [target] + ) + let model = makeWorkspaceObservationUnitModel( + operations: operations, + fileOperations: FailingTrashWorkspaceFileOperations(), + provider: SequencedGitWatchContextProvider([nil]), + watcherFactory: TestDirectoryWatcherFactory(), + refreshGit: {} + ) + model.beginWorkspace(at: workspace, visibilityRules: .default) + _ = await model.rebuild(at: workspace, rules: .default, isCurrent: { true }) + model.requestDeleteProjectItem(at: target, isDirectory: false) + guard let request = model.pendingProjectItemDeletion else { + Issue.record("The deletion request should be available") + return + } + + await model.confirmProjectItemDeletion(request) + + #expect(model.projectFiles == [target]) + #expect(model.rootNode?.children?.map(\.url) == [target]) + } + @Test func workspaceFilesystemFallbackBuildsAVisibleTreeAndHonorsHiddenRules() throws { let fileManager = FileManager.default @@ -3394,7 +3471,8 @@ private func makeWorkspaceObservationUnitModel( watcherFactory: TestDirectoryWatcherFactory, refreshGit: @escaping @MainActor () async -> Void, processExternalChanges: @escaping @MainActor ([URL]) -> Bool = { _ in false }, - reloadProjectServices: @escaping @MainActor () async -> Void = {} + reloadProjectServices: @escaping @MainActor () async -> Void = {}, + recordHistory: @escaping @MainActor (URL, LocalHistoryReason) async -> Void = { _, _ in } ) -> WorkspaceFeatureModel { let model = WorkspaceFeatureModel( operations: operations, @@ -3412,7 +3490,7 @@ private func makeWorkspaceObservationUnitModel( restoreSession: { _, _ in }, openFile: { _ in }, notify: { _ in }, - recordHistory: { _, _ in }, + recordHistory: recordHistory, relocateHistory: { _, _ in }, relocateOpenDocuments: { _, _ in }, closeDocuments: { _ in }, @@ -3725,9 +3803,11 @@ private final class BlockingWorkspaceOperations: WorkspaceOperations, @unchecked private final class SequencedWorkspaceOperations: WorkspaceOperations, @unchecked Sendable { private let lock = NSLock() private var snapshotAvailability: [Bool] + private let files: [URL] - init(snapshotAvailability: [Bool]) { + init(snapshotAvailability: [Bool], files: [URL] = []) { self.snapshotAvailability = snapshotAvailability + self.files = files } func snapshot(at rootURL: URL, visibilityRules: FileVisibilityRules) -> WorkspaceSnapshot? { @@ -3736,8 +3816,12 @@ private final class SequencedWorkspaceOperations: WorkspaceOperations, @unchecke lock.unlock() guard isAvailable else { return nil } return WorkspaceSnapshot( - root: FileNode(url: rootURL, isDirectory: true, children: []), - files: [] + root: FileNode( + url: rootURL, + isDirectory: true, + children: files.map { FileNode(url: $0, isDirectory: false, children: nil) } + ), + files: files ) } @@ -3801,6 +3885,57 @@ private struct EmptyWorkspaceFileOperations: WorkspaceFileOperations { func readText(from url: URL) throws -> String { "" } } +private final class RecordingTrashWorkspaceFileOperations: WorkspaceFileOperations, @unchecked Sendable { + private let lock = NSLock() + private let releaseSemaphore = DispatchSemaphore(value: 0) + private var recordedTrashedURLs: [URL] = [] + private var startedValue = false + + var trashedURLs: [URL] { + lock.withLock { recordedTrashedURLs } + } + + var hasStarted: Bool { + lock.withLock { startedValue } + } + + func release() { + releaseSemaphore.signal() + } + + func fileExists(at url: URL) -> Bool { true } + func isDirectory(at url: URL) -> Bool { false } + func createFile(at url: URL) throws {} + func createDirectory(at url: URL, withIntermediateDirectories: Bool) throws {} + func copyItem(at sourceURL: URL, to destinationURL: URL) throws {} + func moveItem(at sourceURL: URL, to destinationURL: URL) throws {} + func removeItem(at url: URL) throws {} + func trashItem(at url: URL) throws { + lock.withLock { + startedValue = true + } + releaseSemaphore.wait() + lock.withLock { + recordedTrashedURLs.append(url.standardizedFileURL) + } + } + func writeText(_ text: String, to url: URL) throws {} + func readText(from url: URL) throws -> String { "" } +} + +private struct FailingTrashWorkspaceFileOperations: WorkspaceFileOperations { + func fileExists(at url: URL) -> Bool { true } + func isDirectory(at url: URL) -> Bool { false } + func createFile(at url: URL) throws {} + func createDirectory(at url: URL, withIntermediateDirectories: Bool) throws {} + func copyItem(at sourceURL: URL, to destinationURL: URL) throws {} + func moveItem(at sourceURL: URL, to destinationURL: URL) throws {} + func removeItem(at url: URL) throws {} + func trashItem(at url: URL) throws { throw CocoaError(.fileWriteNoPermission) } + func writeText(_ text: String, to url: URL) throws {} + func readText(from url: URL) throws -> String { "" } +} + private final class TestDirectoryChangeSource: DirectoryChangeSource { private let onChange: @Sendable (DirectoryChangeBatch) -> Void