From 9abb43d2d6b42cfd28e8841a958c5afa425fedb1 Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Tue, 21 Jul 2026 13:28:30 +0530 Subject: [PATCH 1/2] Disk Analyzer: act on what the scan finds, and keep the totals true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The analyzer could show you a 40 GB folder and then leave you to go find it yourself. Result rows now carry Reveal in Finder, Copy Path and Move to Trash, on a right-click menu shared by the treemap tiles and the tree pane, and on breadcrumb buttons bound to Finder's own shortcuts so none of it needs a mouse. Deleting goes through FileManager.trashItem and nothing else. A disk analyzer exists to help people decide what to remove, and a wrong decision has to stay recoverable, so there is no path here that unlinks anything. The confirmation names the folder, its full path and the size that is about to move, because the size on a directory row is the whole subtree and that is what goes with it. The part that mattered most was what happens afterwards. Leaving the row on screen would have the analyzer pointing at space it had just freed, so the tree is pruned and every directory above the removed node is rebuilt at its new size. That arithmetic lives in DiskTreeMutator, away from AppKit and the filesystem, because it is what the numbers on screen are made of: it rebuilds only the chain from the root down to the removed node, carries every other subtree over by reference, restores the biggest-first order the treemap and tree pane both read, and floors subtraction at zero so a tree that disagrees with itself can't underflow UInt64 into an 18-exabyte folder. Rebuilding the navigation index after each removal would have meant walking every node on the volume — seconds on a boot disk, with the trashed row still showing for all of it — so DiskTreeCache now stores one parent link per node instead of a materialised ancestor array, which both costs far less memory at millions of nodes and lets the index be patched in place: drop the removed subtree, re-point the children of the rebuilt ancestors, leave everything else alone. The breadcrumb, the expanded-row set and the highlight are moved across the same way, so trashing the folder you are standing in walks you back to its parent instead of stranding you. Three refusals are deliberate. The volume being analyzed is not trashable. The "Unaccounted / Inaccessible" placeholder is not either — it borrows the volume root's path so the treemap can place it, so a Trash aimed at that tile would have been aimed at the mount point; it now carries an isSynthetic flag and the policy checks it. And the command is held back while a scan is still walking that volume, because the scan would keep measuring a directory that had moved out from under it and then commit a tree built from both halves. Every refusal carries the sentence explaining it; a menu item that quietly does nothing is indistinguishable from a bug. An item something else already deleted or moved is not treated as an error at all: existence is probed with lstat rather than FileManager.fileExists, so a dangling symlink still counts as the real directory entry it is, and the same condition is classified back out of whatever trashItem throws in case the item vanishes mid-call. The row is pruned either way, because it was inflating every total above it. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 15 + Sources/DMonteCore/DiskAnalyzer.swift | 457 ++++++++++++++++-- Sources/DMonteCore/DiskItemActions.swift | 209 ++++++++ Sources/DMonteCore/DiskTreeMutation.swift | 190 ++++++++ .../DiskItemActionsTests.swift | 265 ++++++++++ .../DMonteCoreTests/DiskTreeCacheTests.swift | 60 +++ .../DiskTreeMutationTests.swift | 290 +++++++++++ 7 files changed, 1453 insertions(+), 33 deletions(-) create mode 100644 Sources/DMonteCore/DiskItemActions.swift create mode 100644 Sources/DMonteCore/DiskTreeMutation.swift create mode 100644 Tests/DMonteCoreTests/DiskItemActionsTests.swift create mode 100644 Tests/DMonteCoreTests/DiskTreeMutationTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index d4e9936..944eec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,21 @@ adheres to [Semantic Versioning](https://semver.org) and the ## [Unreleased] +### Added +- **Disk Usage Analyzer** can now act on what it finds. Right-click any treemap + block or tree row for Reveal in Finder, Copy Path and Move to Trash; the same + three commands sit in the breadcrumb bar with Finder's own shortcuts (⌘R, ⌥⌘C + and ⌘⌫) so they are reachable without a mouse. Deleting always goes through + the Trash, never a straight unlink, and asks first — naming the folder, its + full path and the size that is about to move. Afterwards the row disappears + and every folder above it shrinks by exactly what left, so the totals stay + true instead of pointing at space that has already been freed. A row whose + file something else already deleted or moved says so plainly and is dropped + from the results rather than failing silently, the volume being analyzed and + the "Unaccounted / Inaccessible" estimate are never trashable, and the command + is held back while a scan is still walking that volume so a half-measured + folder can't be pulled out from under it. + ## [0.14.0] — 2026-07-21 ### Added diff --git a/Sources/DMonteCore/DiskAnalyzer.swift b/Sources/DMonteCore/DiskAnalyzer.swift index cd9ff59..515081c 100644 --- a/Sources/DMonteCore/DiskAnalyzer.swift +++ b/Sources/DMonteCore/DiskAnalyzer.swift @@ -80,6 +80,12 @@ public final class DiskNode: Identifiable, Sendable { public let isDirectory: Bool public let size: UInt64 public let children: [DiskNode] + /// True only for the reconciliation placeholder, which stands for bytes the + /// volume reports but the walk could not attribute to any file. It borrows the + /// volume root's path to be placeable in the treemap, so anything that acts on + /// a node's path — reveal, copy, and above all Move to Trash — has to be able + /// to tell it apart from a real row (see DiskItemActionPolicy). + public let isSynthetic: Bool public var id: ObjectIdentifier { ObjectIdentifier(self) } @@ -87,12 +93,20 @@ public final class DiskNode: Identifiable, Sendable { URL(fileURLWithPath: path) } - public init(path: String, name: String, isDirectory: Bool, size: UInt64, children: [DiskNode]) { + public init( + path: String, + name: String, + isDirectory: Bool, + size: UInt64, + children: [DiskNode], + isSynthetic: Bool = false + ) { self.path = path self.name = name self.isDirectory = isDirectory self.size = size self.children = children + self.isSynthetic = isSynthetic } public var fileExtension: String { @@ -294,7 +308,10 @@ public enum DiskScanner { return reconciled(root: root, usedBytes: volume.usedBytes) } - private static func reconciled(root: DiskNode, usedBytes: UInt64) -> DiskNode { + /// Internal rather than private so a test can pin the one property that keeps a + /// Move to Trash away from the volume root: the placeholder this synthesises + /// borrows the root's path, and only its `isSynthetic` flag tells it apart. + static func reconciled(root: DiskNode, usedBytes: UInt64) -> DiskNode { guard usedBytes > root.size else { return root } let gap = usedBytes - root.size guard gap > usedBytes / 200 else { return root } @@ -304,7 +321,8 @@ public enum DiskScanner { name: unaccountedNodeName, isDirectory: false, size: gap, - children: [] + children: [], + isSynthetic: true ) let children = (root.children + [placeholder]).sorted { $0.size > $1.size } return DiskNode(path: root.path, name: root.name, isDirectory: true, size: usedBytes, children: children) @@ -535,38 +553,115 @@ public struct TreemapRect: Identifiable, Sendable { struct DiskTreeCache { let root: DiskNode - let pathByNodeID: [ObjectIdentifier: [DiskNode]] - let nodeCount: Int + /// One parent reference per node, walked upward on demand by `path(to:)`. + /// Storing links rather than a materialised ancestor array per node matters at + /// this scale — a boot volume indexes millions of nodes — and it is also what + /// makes `applying(_:)` possible: pruning a node leaves every link outside the + /// rebuilt spine still correct. + private let parentByNodeID: [ObjectIdentifier: DiskNode] + /// Membership of the tree, tracked separately because the root has no parent + /// link. Without it `path(to:)` could not tell the root from a node that was + /// never indexed at all. + private let indexedNodeIDs: Set + + var nodeCount: Int { indexedNodeIDs.count } init(root: DiskNode) { - var paths: [ObjectIdentifier: [DiskNode]] = [:] - var count = 0 - Self.index(node: root, path: [], paths: &paths, count: &count) + var parents: [ObjectIdentifier: DiskNode] = [:] + var indexed: Set = [] + Self.index(node: root, parent: nil, parents: &parents, indexed: &indexed) + self.root = root + self.parentByNodeID = parents + self.indexedNodeIDs = indexed + } + + private init( + root: DiskNode, + parentByNodeID: [ObjectIdentifier: DiskNode], + indexedNodeIDs: Set + ) { self.root = root - self.pathByNodeID = paths - self.nodeCount = count + self.parentByNodeID = parentByNodeID + self.indexedNodeIDs = indexedNodeIDs } func path(to node: DiskNode) -> [DiskNode]? { - pathByNodeID[node.id] + guard indexedNodeIDs.contains(node.id) else { return nil } + + var chain: [DiskNode] = [node] + var cursor = node + while let parent = parentByNodeID[cursor.id] { + chain.append(parent) + cursor = parent + // A scanned tree is acyclic so this always terminates at the root; the + // bound only makes a corrupted map fail loudly instead of hanging. + if chain.count > indexedNodeIDs.count { return nil } + } + + return chain.reversed() + } + + /// Returns the index updated for a node that has just left the tree, instead of + /// rebuilding it from scratch. A rebuild walks every node on the volume — seconds + /// on a boot disk — and the tree pane would have to show the trashed row for the + /// whole of it. Only two things actually changed: the removed subtree is no longer + /// part of the tree, and each of its ancestors was rebuilt at a smaller size, so + /// the children of those rebuilt directories need re-pointing. Every other node + /// kept its identity and its parent, so its link is still true. + func applying(_ removal: DiskTreeRemoval) -> DiskTreeCache { + var parents = parentByNodeID + var indexed = indexedNodeIDs + + Self.forget(node: removal.removed, parents: &parents, indexed: &indexed) + + // Root first, so each rebuilt directory has already re-pointed the next one + // by the time we reach it. + for replacement in removal.replacedAncestors { + parents.removeValue(forKey: replacement.old.id) + indexed.remove(replacement.old.id) + indexed.insert(replacement.new.id) + + for child in replacement.new.children { + parents[child.id] = replacement.new + } + } + + // The new root is nobody's child; a stale link here would send path(to:) + // walking back into the tree we just replaced. + parents.removeValue(forKey: removal.root.id) + + return DiskTreeCache(root: removal.root, parentByNodeID: parents, indexedNodeIDs: indexed) } private static func index( node: DiskNode, - path: [DiskNode], - paths: inout [ObjectIdentifier: [DiskNode]], - count: inout Int + parent: DiskNode?, + parents: inout [ObjectIdentifier: DiskNode], + indexed: inout Set ) { // The cache is built on a detached task (see scheduleTreeCacheBuild) because a // boot volume holds millions of nodes. Bail per-node when that task is // cancelled — a superseded build's partial result is discarded by its caller, // so there is no point finishing the walk. if Task.isCancelled { return } - count += 1 - let currentPath = path + [node] - paths[node.id] = currentPath + indexed.insert(node.id) + if let parent { + parents[node.id] = parent + } + for child in node.children { + index(node: child, parent: node, parents: &parents, indexed: &indexed) + } + } + + private static func forget( + node: DiskNode, + parents: inout [ObjectIdentifier: DiskNode], + indexed: inout Set + ) { + indexed.remove(node.id) + parents.removeValue(forKey: node.id) for child in node.children { - index(node: child, path: currentPath, paths: &paths, count: &count) + forget(node: child, parents: &parents, indexed: &indexed) } } } @@ -755,6 +850,14 @@ public enum DiskItemPalette { } } +/// A one-line result of a row action. Equatable (and free of any identifier) so the +/// auto-dismiss timer can key off the message itself: an identical message posted +/// twice keeps one timer, a different one restarts the clock. +private struct DiskActionStatus: Equatable { + var text: String + var isError: Bool +} + public struct DiskAnalyzerWindowView: View { var onQuit: () -> Void @@ -775,6 +878,12 @@ public struct DiskAnalyzerWindowView: View { @State private var scanGates: [String: ScanPauseGate] = [:] @State private var pausedVolumeIds: Set = [] @State private var hoveredNode: DiskNode? + /// The row the toolbar buttons and their keyboard shortcuts act on — set by + /// clicking a tile or a tree row. Kept apart from `hoveredNode`, which follows + /// the pointer and would make ⌘⌫ act on whatever the mouse happened to be over. + @State private var focusedNode: DiskNode? + @State private var actionStatus: DiskActionStatus? + @State private var isTrashInFlight = false @State private var isShowingSettings = false @State private var fullDiskAccessGranted = true @State private var isFullScreen = false @@ -794,6 +903,9 @@ public struct DiskAnalyzerWindowView: View { volumeStrip summaryPanel breadcrumbBar + if let actionStatus { + actionStatusBanner(actionStatus) + } if showsFullDiskAccessHint { fullDiskAccessHint } @@ -1039,11 +1151,93 @@ public struct DiskAnalyzerWindowView: View { } } - Spacer(minLength: 0) + Spacer(minLength: 8) + + rowActionBar } .frame(height: layout.breadcrumbHeight) } + /// The keyboard-reachable half of the row actions. The context menu is the + /// discoverable route, but a menu that only opens on right-click is unusable + /// without a mouse — these buttons carry the same three commands with Finder's + /// own shortcuts, and they are what makes the actions reachable by keyboard and + /// visible to VoiceOver. + private var rowActionBar: some View { + HStack(spacing: layout.breadcrumbSpacing) { + rowActionButton(.revealInFinder, systemImage: "folder", help: "Reveal in Finder") + .keyboardShortcut("r", modifiers: [.command]) + + rowActionButton(.copyPath, systemImage: "doc.on.doc", help: "Copy Path") + .keyboardShortcut("c", modifiers: [.command, .option]) + + rowActionButton(.moveToTrash, systemImage: "trash", help: "Move to Trash") + .keyboardShortcut(.delete, modifiers: [.command]) + } + } + + private func rowActionButton(_ action: DiskItemAction, systemImage: String, help: String) -> some View { + Button { + perform(action, on: focusedNode) + } label: { + Image(systemName: systemImage) + .font(.system(size: layout.breadcrumbIconSize, weight: .bold)) + .foregroundStyle(.secondary) + .frame(width: layout.breadcrumbButtonSize, height: layout.breadcrumbButtonSize) + .background(Color.secondary.opacity(0.12)) + .clipShape(RoundedRectangle(cornerRadius: 6, style: .continuous)) + } + .buttonStyle(.plain) + .disabled(focusedNode == nil) + .opacity(focusedNode == nil ? 0.4 : 1) + .help(focusedNode.map { "\(help) — \($0.name)" } ?? "\(help) (select an item first)") + .accessibilityLabel(help) + } + + private func actionStatusBanner(_ status: DiskActionStatus) -> some View { + HStack(spacing: 8) { + Image(systemName: status.isError ? "exclamationmark.triangle.fill" : "checkmark.circle.fill") + .font(.system(size: layout.breadcrumbFontSize, weight: .semibold)) + .foregroundStyle(status.isError ? Color.orange : Color.accentColor) + + Text(status.text) + .font(.system(size: layout.breadcrumbFontSize - 1, weight: .medium)) + .foregroundStyle(.secondary) + .lineLimit(2) + .fixedSize(horizontal: false, vertical: true) + .textSelection(.enabled) + + Spacer(minLength: 8) + + Button { + actionStatus = nil + } label: { + Image(systemName: "xmark") + .font(.system(size: layout.breadcrumbFontSize - 2, weight: .bold)) + .foregroundStyle(.secondary) + } + .buttonStyle(.plain) + .help("Dismiss") + } + .padding(.horizontal, 12) + .padding(.vertical, 8) + .frame(maxWidth: .infinity, alignment: .leading) + .background((status.isError ? Color.orange : Color.accentColor).opacity(0.12)) + .clipShape(RoundedRectangle(cornerRadius: 8, style: .continuous)) + .overlay { + RoundedRectangle(cornerRadius: 8, style: .continuous) + .strokeBorder((status.isError ? Color.orange : Color.accentColor).opacity(0.35), lineWidth: 0.5) + } + .task(id: status) { + // Transient by design: long enough to read, then out of the way. Keying + // the task on the status means a newer message restarts the clock rather + // than inheriting whatever was left of the previous one's. + try? await Task.sleep(for: .seconds(8)) + guard !Task.isCancelled, actionStatus == status else { return } + actionStatus = nil + } + } + private var fullDiskAccessHint: some View { HStack(spacing: 8) { Image(systemName: "lock.shield") @@ -1113,14 +1307,10 @@ public struct DiskAnalyzerWindowView: View { hoveredNode = nil } } - .onTapGesture(count: 2) { revealInFinder(node: entry.node) } + .onTapGesture(count: 2) { perform(.revealInFinder, on: entry.node) } .onTapGesture { handleTap(node: entry.node) } .contextMenu { - Button { - revealInFinder(node: entry.node) - } label: { - Label("Show in Finder", systemImage: "folder") - } + DiskItemActionMenu { perform($0, on: entry.node) } } .help(tooltip(for: entry.node)) // Use .position (not .frame+.offset): offset moves only the @@ -1199,7 +1389,7 @@ public struct DiskAnalyzerWindowView: View { layout: layout, onToggleExpand: toggleTreeExpansion, onSelect: navigateFromTree, - onReveal: revealInFinder + onAction: { perform($0, on: $1) } ) } .padding(.vertical, 4) @@ -1365,6 +1555,7 @@ public struct DiskAnalyzerWindowView: View { self.selectedVolume = nil pathStack = [] hoveredNode = nil + focusedNode = nil } if autoSelectFirst, selectedVolume == nil, let first = detected.first { @@ -1378,6 +1569,8 @@ public struct DiskAnalyzerWindowView: View { // the pause button, so background scans keep running unless the user // explicitly pauses them. hoveredNode = nil + focusedNode = nil + actionStatus = nil selectedVolume = volume if let cached = treeCaches[volume.id] { @@ -1415,6 +1608,10 @@ public struct DiskAnalyzerWindowView: View { let volumeId = volume.id teardownScan(for: volumeId) hoveredNode = nil + // Both point into the tree this scan is about to replace, and the status + // line refers to a row that will no longer exist. + focusedNode = nil + actionStatus = nil fullDiskAccessGranted = FullDiskAccess.isGranted() let generation = (scanGenerations[volumeId] ?? 0) + 1 @@ -1561,6 +1758,7 @@ public struct DiskAnalyzerWindowView: View { private func handleTap(node: DiskNode) { expandTreePath(to: node) scrollTree(to: node) + focusedNode = node guard node.isDirectory, !node.children.isEmpty else { hoveredNode = node @@ -1588,6 +1786,7 @@ public struct DiskAnalyzerWindowView: View { pathStack = Array(path.dropLast()) } hoveredNode = node + focusedNode = node } private func toggleTreeExpansion(node: DiskNode) { @@ -1633,10 +1832,171 @@ public struct DiskAnalyzerWindowView: View { hoveredNode = nil } + /// True while nothing on the selected volume can safely be trashed: either the + /// scan is still walking it, or its result is still being indexed and there is + /// no settled tree to prune the row out of afterwards. + private var isTreeUnsettled: Bool { + guard let id = selectedVolume?.id else { return true } + return scanTasks[id] != nil || treeCacheBuildTasks[id] != nil || treeCaches[id] == nil + } + + private func availability(of action: DiskItemAction, for node: DiskNode) -> DiskItemActionAvailability { + DiskItemActionPolicy.availability( + of: action, + for: node, + isScanRoot: selectedTreeCache?.root.id == node.id || pathStack.first?.id == node.id, + isScanInProgress: isTreeUnsettled || isTrashInFlight + ) + } + + private func perform(_ action: DiskItemAction, on node: DiskNode?) { + guard let node else { + // Only reachable from the toolbar buttons, which are disabled until a + // row is picked; say so rather than doing nothing. + actionStatus = DiskActionStatus(text: "Select an item in the treemap or the tree first.", isError: true) + return + } + + let availability = availability(of: action, for: node) + guard availability.isAllowed else { + actionStatus = DiskActionStatus(text: availability.reason ?? "", isError: true) + return + } + + switch action { + case .revealInFinder: + revealInFinder(node: node) + case .copyPath: + copyPath(of: node) + case .moveToTrash: + moveToTrash(node: node) + } + } + private func revealInFinder(node: DiskNode) { + // activateFileViewerSelecting silently does nothing for a path that has gone + // away, which would look like a broken menu item, so check first. + guard DiskItemProbe.exists(atPath: node.path) else { + actionStatus = DiskActionStatus( + text: "\(node.name) is no longer at \(node.path). Rescan to refresh the results.", + isError: true + ) + return + } NSWorkspace.shared.activateFileViewerSelecting([node.url]) } + private func copyPath(of node: DiskNode) { + // Deliberately not gated on the file still existing: the path of something + // that just vanished is exactly what someone pastes into a shell to go + // looking for it. + if DiskItemPathCopier.copy(path: node.path) { + actionStatus = DiskActionStatus(text: DiskItemActionMessage.pathCopied(node.path), isError: false) + } else { + actionStatus = DiskActionStatus(text: DiskItemActionMessage.pathCopyFailed(name: node.name), isError: true) + } + } + + private func moveToTrash(node: DiskNode) { + guard let volumeId = selectedVolume?.id else { return } + + let prompt = DiskItemActionMessage.trashPrompt(for: node) + let alert = NSAlert() + alert.messageText = prompt.title + alert.informativeText = prompt.detail + // Critical rather than warning: a row in this window can be hundreds of + // gigabytes, and the folder case takes everything underneath it. + alert.alertStyle = node.isDirectory ? .critical : .warning + alert.addButton(withTitle: "Move to Trash") + alert.addButton(withTitle: "Cancel") + + guard alert.runModal() == .alertFirstButtonReturn else { return } + + // Trashing is a filesystem round trip; keep it off the main actor so a slow + // volume doesn't freeze the window, and latch a flag so the confirmation + // can't be answered twice for the same row while it is in flight. + isTrashInFlight = true + let path = node.path + Task { + let result = await Task.detached(priority: .userInitiated) { + DiskItemTrasher.moveToTrash(path: path) + }.value + + isTrashInFlight = false + + switch result { + case .success: + let freed = node.size + prune(node: node, in: volumeId) + actionStatus = DiskActionStatus( + text: DiskItemActionMessage.trashSucceeded(name: node.name, bytes: freed), + isError: false + ) + case .failure(.alreadyGone): + // Nothing went wrong — the scan was simply older than the disk. The + // row still has to go, because it is describing something that is + // not there and is inflating every total above it. + prune(node: node, in: volumeId) + actionStatus = DiskActionStatus( + text: DiskItemActionMessage.trashFailed(name: node.name, failure: .alreadyGone), + isError: false + ) + case .failure(let failure): + // The item is still on disk, so the tree stays exactly as it is. + actionStatus = DiskActionStatus( + text: DiskItemActionMessage.trashFailed(name: node.name, failure: failure), + isError: true + ) + } + } + } + + /// Takes a node that is no longer on disk out of the displayed tree and shrinks + /// every directory above it, so the totals keep matching reality instead of + /// quietly counting bytes that have been freed. + /// + /// Keyed by volume rather than by "whatever is selected": trashing is + /// asynchronous, and the user may well have clicked another volume's chip while + /// the Trash was busy. The tree that has to be corrected is the one the row came + /// from, whether or not it is the one on screen. + private func prune(node: DiskNode, in volumeId: String) { + guard let cache = treeCaches[volumeId], + let path = cache.path(to: node), + let removal = DiskTreeMutator.removing(nodeAt: path) else { + return + } + + // Patching the index instead of rebuilding it keeps the tree pane populated; + // a rebuild walks every node on the volume and would show the trashed row + // for the whole of that walk. + treeCaches[volumeId] = cache.applying(removal) + + let replacements = DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors) + expandedTreeNodeIDs = DiskTreeNavigationPatch.patchedExpansion( + expandedTreeNodeIDs, + using: removal.replacedAncestors + ) + + // The breadcrumb, the hover chip and the focused row all describe the volume + // currently on screen; leave them alone when the pruned tree is a different + // one, or they would start pointing into a tree nobody is looking at. + if selectedVolume?.id == volumeId { + pathStack = DiskTreeNavigationPatch.patchedPath( + pathStack, + removing: removal.removed, + using: replacements, + fallbackRoot: removal.root + ) + hoveredNode = DiskTreeNavigationPatch.patchedSelection(hoveredNode, removing: removal.removed, using: replacements) + focusedNode = DiskTreeNavigationPatch.patchedSelection(focusedNode, removing: removal.removed, using: replacements) + } + + // Treemap layouts are keyed by node identity, so the rebuilt spine simply + // misses the cache and is recomputed on demand until this lands — stale, but + // never wrong. Rebuilding also drops the now-unreachable old entries. + scheduleTreemapLayoutCache(for: volumeId, root: removal.root) + } + private func requestQuit() { for task in scanTasks.values { task.cancel() @@ -1651,6 +2011,36 @@ public struct DiskAnalyzerWindowView: View { } } +/// The row actions, shared verbatim by the treemap tiles and the tree pane so the +/// two never drift apart. Nothing here is disabled: an action that isn't allowed +/// for this row explains why when it is chosen (see DiskItemActionPolicy), which +/// is far more useful than a greyed-out item with no reason attached. +private struct DiskItemActionMenu: View { + var perform: (DiskItemAction) -> Void + + var body: some View { + Button { + perform(.revealInFinder) + } label: { + Label("Reveal in Finder", systemImage: "folder") + } + + Button { + perform(.copyPath) + } label: { + Label("Copy Path", systemImage: "doc.on.doc") + } + + Divider() + + Button(role: .destructive) { + perform(.moveToTrash) + } label: { + Label("Move to Trash", systemImage: "trash") + } + } +} + private struct VolumeChip: View { var volume: DiskVolume var isSelected: Bool @@ -1704,7 +2094,7 @@ private struct DiskTreeRow: View { var layout: DiskAnalyzerLayout var onToggleExpand: (DiskNode) -> Void var onSelect: (DiskNode) -> Void - var onReveal: (DiskNode) -> Void + var onAction: (DiskItemAction, DiskNode) -> Void var body: some View { VStack(alignment: .leading, spacing: 0) { @@ -1758,11 +2148,7 @@ private struct DiskTreeRow: View { onSelect(node) } .contextMenu { - Button { - onReveal(node) - } label: { - Label("Show in Finder", systemImage: "folder") - } + DiskItemActionMenu { onAction($0, node) } } } .padding(.trailing, 8) @@ -1780,7 +2166,7 @@ private struct DiskTreeRow: View { layout: layout, onToggleExpand: onToggleExpand, onSelect: onSelect, - onReveal: onReveal + onAction: onAction ) } } @@ -2176,6 +2562,11 @@ private struct DiskAnalyzerSettingsView: View { .foregroundStyle(.secondary) .fixedSize(horizontal: false, vertical: true) + Text("Right-click any block or tree row for Reveal in Finder (\u{2318}R), Copy Path (\u{2325}\u{2318}C) and Move to Trash (\u{2318}\u{232B}). Trashed items go to the Trash, never straight to deletion, and the totals above shrink to match.") + .font(.system(size: 12, weight: .medium)) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + Button(role: .destructive) { onClose() onQuit() diff --git a/Sources/DMonteCore/DiskItemActions.swift b/Sources/DMonteCore/DiskItemActions.swift new file mode 100644 index 0000000..5a6b2fc --- /dev/null +++ b/Sources/DMonteCore/DiskItemActions.swift @@ -0,0 +1,209 @@ +import AppKit +import Foundation + +/// The three things the Disk Usage Analyzer offers to do with a row of scan +/// results. Modelled as a value rather than three separate call sites so the +/// availability rules below can be stated once and tested once. +public enum DiskItemAction: Sendable, CaseIterable { + case revealInFinder + case copyPath + case moveToTrash +} + +/// Whether an action may run against a particular row, and when it may not, the +/// sentence to show the user. Every refusal carries a reason because the +/// alternative — a menu item that quietly does nothing — is indistinguishable +/// from a bug. +public enum DiskItemActionAvailability: Equatable, Sendable { + case allowed + case unavailable(reason: String) + + public var isAllowed: Bool { + self == .allowed + } + + public var reason: String? { + guard case .unavailable(let reason) = self else { return nil } + return reason + } +} + +/// Decides which row actions are safe to offer. Kept pure and separate from the +/// view because two of these rules stop real damage: the reconciliation +/// placeholder would aim a Trash command at the volume root, and a row trashed +/// mid-scan leaves the running scan measuring a directory that no longer exists. +public enum DiskItemActionPolicy { + public static func availability( + of action: DiskItemAction, + for node: DiskNode, + isScanRoot: Bool, + isScanInProgress: Bool + ) -> DiskItemActionAvailability { + // The "Unaccounted / Inaccessible" node carries the volume root's path so + // the treemap can place it, but it stands for bytes macOS reports that we + // could not attribute to any file. Acting on it would act on the whole + // volume — trashing it would target the mount point — so nothing is offered. + if node.isSynthetic { + return .unavailable( + reason: "\(node.name) is an estimate of space we couldn't attribute to a file, so there is nothing to act on." + ) + } + + switch action { + case .revealInFinder, .copyPath: + // Both are read-only and stay useful mid-scan, so the only bar is the + // synthetic-node check above. + return .allowed + + case .moveToTrash: + if isScanRoot { + return .unavailable( + reason: "\(node.name) is the volume being analyzed, so it can't be moved to the Trash from here." + ) + } + if isScanInProgress { + // A scan already walking this volume would keep measuring the + // directory we just moved, and would then commit a tree built from + // a mix of before and after. Finishing the scan first is cheaper + // than trying to patch a half-built tree. + return .unavailable( + reason: "A scan is still running on this volume. Let it finish before moving anything to the Trash." + ) + } + return .allowed + } + } +} + +/// Why a Move to Trash did not happen. Split into exactly the two cases the UI +/// treats differently: an item that is already gone is not an error the user has +/// to fix — the scan results were simply stale — so the row still disappears. +public enum DiskItemTrashFailure: Error, Equatable, Sendable { + case alreadyGone + case systemRefused(String) +} + +public enum DiskItemProbe { + /// Whether the item a scan row describes is still there. + /// + /// `lstat`, not `FileManager.fileExists`, which follows symlinks and would + /// therefore call a dangling link "gone" even though the link itself is a real + /// directory entry the user can see, reveal and trash. + public static func exists(atPath path: String) -> Bool { + guard !path.isEmpty else { return false } + var info = stat() + return URL(fileURLWithPath: path).withUnsafeFileSystemRepresentation { representation in + guard let representation else { return false } + return lstat(representation, &info) == 0 + } + } +} + +public enum DiskItemTrasher { + /// Moves the item at `path` to the Trash. + /// + /// Always `FileManager.trashItem`, never `removeItem`/`unlink`: a disk + /// analyzer exists to help people decide what to delete, and a wrong decision + /// has to stay recoverable. The Trash is that undo. + public static func moveToTrash(path: String, fileManager: FileManager = .default) -> Result { + let url = URL(fileURLWithPath: path) + + // Scan results can be minutes old, so the row may be describing something + // that Finder or another process already removed. Check first, and map the + // same condition out of the thrown error below, because the item can also + // vanish between this check and the call. + guard DiskItemProbe.exists(atPath: path) else { + return .failure(.alreadyGone) + } + + do { + var resultingURL: NSURL? + try fileManager.trashItem(at: url, resultingItemURL: &resultingURL) + return .success(()) + } catch { + return .failure(failure(from: error)) + } + } + + /// Classifies an error thrown by `trashItem`. Exposed rather than inlined into + /// the `catch` so the "it disappeared while we were asking" race can be pinned + /// by a test without a filesystem to race against. + public static func failure(from error: Error) -> DiskItemTrashFailure { + let nsError = error as NSError + + if nsError.domain == NSCocoaErrorDomain, + nsError.code == NSFileNoSuchFileError || nsError.code == NSFileReadNoSuchFileError { + return .alreadyGone + } + + if nsError.domain == NSPOSIXErrorDomain, nsError.code == Int(ENOENT) { + return .alreadyGone + } + + // Anything else — permissions, a read-only volume, a locked file — is a + // real refusal the user needs to read, so it is passed through verbatim. + return .systemRefused(nsError.localizedDescription) + } +} + +/// The two lines of a Move to Trash confirmation. A struct rather than a tuple so +/// tests can compare the whole prompt in one assertion. +public struct DiskTrashPrompt: Equatable, Sendable { + public let title: String + public let detail: String + + public init(title: String, detail: String) { + self.title = title + self.detail = detail + } +} + +/// Every sentence the row actions can put in front of the user, in one place so +/// the wording is testable and cannot drift between the menu, the alert and the +/// status line. +public enum DiskItemActionMessage { + public static func trashPrompt(for node: DiskNode) -> DiskTrashPrompt { + // Directories are the dangerous case: the size on the row is the whole + // subtree, and that is exactly what goes to the Trash, so say so. + let detail = node.isDirectory + ? "\(node.path)\n\nThis folder and everything inside it — \(node.size.diskBytesString) — will be moved to the Trash." + : "\(node.path)\n\n\(node.size.diskBytesString) will be moved to the Trash." + + return DiskTrashPrompt(title: "Move \u{201C}\(node.name)\u{201D} to the Trash?", detail: detail) + } + + public static func trashSucceeded(name: String, bytes: UInt64) -> String { + "Moved \(name) to the Trash and freed \(bytes.diskBytesString) from the totals." + } + + public static func trashFailed(name: String, failure: DiskItemTrashFailure) -> String { + switch failure { + case .alreadyGone: + // Not phrased as an error: nothing went wrong, the scan was just old. + // The caller still prunes the row, and this explains why it vanished. + return "\(name) was already deleted or moved, so there was nothing to trash. Removed it from the results." + case .systemRefused(let reason): + return "Couldn't move \(name) to the Trash: \(reason)" + } + } + + public static func pathCopied(_ path: String) -> String { + "Copied \(path) to the clipboard." + } + + public static func pathCopyFailed(name: String) -> String { + "Couldn't copy the path for \(name) to the clipboard." + } +} + +public enum DiskItemPathCopier { + /// Puts a row's full path on the clipboard. `pasteboard` is injectable so the + /// contract can be asserted against a private pasteboard instead of trampling + /// the user's real clipboard during tests. + @discardableResult + public static func copy(path: String, to pasteboard: NSPasteboard = .general) -> Bool { + guard !path.isEmpty else { return false } + pasteboard.clearContents() + return pasteboard.setString(path, forType: .string) + } +} diff --git a/Sources/DMonteCore/DiskTreeMutation.swift b/Sources/DMonteCore/DiskTreeMutation.swift new file mode 100644 index 0000000..9405044 --- /dev/null +++ b/Sources/DMonteCore/DiskTreeMutation.swift @@ -0,0 +1,190 @@ +import Foundation + +/// One ancestor of a removed node, paired with the rebuilt node that replaces it. +/// +/// `DiskNode` is immutable, so shrinking a directory means constructing a new one; +/// callers that key state off node identity (the expanded-rows set, the breadcrumb +/// stack, the treemap layout cache) need this pairing to carry that state across. +public struct DiskNodeReplacement: Sendable { + public let old: DiskNode + public let new: DiskNode + + public init(old: DiskNode, new: DiskNode) { + self.old = old + self.new = new + } +} + +/// The result of pruning one node out of a scanned tree: a new root whose every +/// affected directory has already been resized, plus the ancestor pairings needed +/// to move UI state onto it. +public struct DiskTreeRemoval: Sendable { + public let root: DiskNode + public let removed: DiskNode + /// Root first, immediate parent last — the order the tree cache relies on when + /// it re-points children at their rebuilt parents. + public let replacedAncestors: [DiskNodeReplacement] + + public init(root: DiskNode, removed: DiskNode, replacedAncestors: [DiskNodeReplacement]) { + self.root = root + self.removed = removed + self.replacedAncestors = replacedAncestors + } +} + +/// Pure tree surgery for "this item is no longer on disk". +/// +/// Kept entirely free of AppKit, the filesystem and the view so the arithmetic that +/// keeps the displayed totals honest can be tested directly. Only the chain from the +/// root down to the removed node is rebuilt; every other subtree is carried over by +/// reference, which keeps the operation proportional to the tree's depth rather than +/// its size and lets identity-keyed caches survive untouched. +public enum DiskTreeMutator { + /// Removes `node` from the tree rooted at `root`, shrinking every ancestor. + /// Returns nil when `node` is the root itself (a scan has to keep its root) or + /// is not in this tree at all. + public static func removing(_ node: DiskNode, from root: DiskNode) -> DiskTreeRemoval? { + guard let path = ancestorPath(to: node, from: root) else { return nil } + return removing(nodeAt: path) + } + + /// Removes the last node of `path`, where `path` runs from the tree's root down + /// to the doomed node. The view takes this overload because its tree index + /// already knows every node's ancestry, so the search below can be skipped. + public static func removing(nodeAt path: [DiskNode]) -> DiskTreeRemoval? { + // A single-element path is the root, which has no parent to remove it from. + guard path.count >= 2, let removed = path.last else { return nil } + + var replacements: [DiskNodeReplacement] = [] + var child = removed + // Nil until the first rebuild, which is what distinguishes "drop this child" + // (the removed node) from "swap this child for its resized rebuild". + var rebuiltChild: DiskNode? + + for index in stride(from: path.count - 2, through: 0, by: -1) { + let parent = path[index] + guard let slot = parent.children.firstIndex(where: { $0 === child }) else { + // The supplied path does not describe this tree; refuse rather than + // hand back a tree whose sizes no longer add up. + return nil + } + + var children = parent.children + if let rebuiltChild { + children[slot] = rebuiltChild + } else { + children.remove(at: slot) + } + // The scanner hands back children biggest-first, and both the treemap's + // squarify pass and the tree pane read that order as meaningful. A + // shrunken directory can easily be smaller than the sibling that used to + // sit below it, so the order is restored rather than left lying. + children.sort { $0.size > $1.size } + + let rebuiltParent = DiskNode( + path: parent.path, + name: parent.name, + isDirectory: parent.isDirectory, + size: reducing(parent.size, by: removed.size), + children: children, + isSynthetic: parent.isSynthetic + ) + + replacements.append(DiskNodeReplacement(old: parent, new: rebuiltParent)) + child = parent + rebuiltChild = rebuiltParent + } + + guard let newRoot = rebuiltChild else { return nil } + + return DiskTreeRemoval( + root: newRoot, + removed: removed, + replacedAncestors: replacements.reversed() + ) + } + + /// The chain of nodes from `root` down to `node`, or nil if `node` is not in the + /// tree. Matching is by object identity, not by path string: the reconciliation + /// placeholder deliberately shares the root's path, so a string comparison could + /// pick the wrong node. + static func ancestorPath(to node: DiskNode, from root: DiskNode) -> [DiskNode]? { + if node === root { return [root] } + + for child in root.children { + if let tail = ancestorPath(to: node, from: child) { + return [root] + tail + } + } + + return nil + } + + /// A directory's size is the sum of what the scanner attributed to its children, + /// so subtracting the removed subtree is exact. The floor is only there so a tree + /// that somehow disagrees with itself can't wrap UInt64 around into an + /// astronomically wrong total on screen. + private static func reducing(_ size: UInt64, by amount: UInt64) -> UInt64 { + size > amount ? size - amount : 0 + } +} + +/// Moves the analyzer's identity-keyed navigation state onto a tree that has just +/// had a node pruned out of it. Pure so the awkward cases — trashing the folder you +/// are currently looking at, trashing an ancestor of the highlighted row — are +/// testable without a running view. +enum DiskTreeNavigationPatch { + static func replacementMap(for replacements: [DiskNodeReplacement]) -> [ObjectIdentifier: DiskNode] { + var map: [ObjectIdentifier: DiskNode] = [:] + for replacement in replacements { + map[replacement.old.id] = replacement.new + } + return map + } + + /// The new breadcrumb stack. If the removed node is on the current path then the + /// user was standing inside (or on) something that no longer exists, so the walk + /// stops at its parent; otherwise the stack keeps its shape and only swaps in the + /// rebuilt ancestors. + static func patchedPath( + _ stack: [DiskNode], + removing removed: DiskNode, + using replacements: [ObjectIdentifier: DiskNode], + fallbackRoot: DiskNode + ) -> [DiskNode] { + var kept = stack + if let index = kept.firstIndex(where: { $0 === removed }) { + kept = Array(kept.prefix(index)) + } + + let patched = kept.map { replacements[$0.id] ?? $0 } + // The stack must never be empty: it is what the treemap draws from. + return patched.isEmpty ? [fallbackRoot] : patched + } + + /// Carries the expanded-rows set across the rebuild, so a folder the user had + /// open does not silently collapse because its object was replaced. + static func patchedExpansion( + _ expanded: Set, + using replacements: [DiskNodeReplacement] + ) -> Set { + var patched = expanded + for replacement in replacements where patched.contains(replacement.old.id) { + patched.remove(replacement.old.id) + patched.insert(replacement.new.id) + } + return patched + } + + /// The hovered/focused row after the rebuild: gone if it was the node we removed, + /// otherwise remapped so the highlight follows a rebuilt ancestor. + static func patchedSelection( + _ selection: DiskNode?, + removing removed: DiskNode, + using replacements: [ObjectIdentifier: DiskNode] + ) -> DiskNode? { + guard let selection else { return nil } + if selection === removed { return nil } + return replacements[selection.id] ?? selection + } +} diff --git a/Tests/DMonteCoreTests/DiskItemActionsTests.swift b/Tests/DMonteCoreTests/DiskItemActionsTests.swift new file mode 100644 index 0000000..8c6584b --- /dev/null +++ b/Tests/DMonteCoreTests/DiskItemActionsTests.swift @@ -0,0 +1,265 @@ +import AppKit +import XCTest +@testable import DMonteCore + +final class DiskItemActionsTests: XCTestCase { + private func file(name: String = "big.bin", path: String = "/Volumes/Data/big.bin", size: UInt64 = 4096) -> DiskNode { + DiskNode(path: path, name: name, isDirectory: false, size: size, children: []) + } + + private func folder(name: String = "Renders", path: String = "/Volumes/Data/Renders", size: UInt64 = 1_073_741_824) -> DiskNode { + DiskNode(path: path, name: name, isDirectory: true, size: size, children: []) + } + + // MARK: - Policy + + func testAnOrdinaryRowOnASettledTreeAllowsAllThreeActions() { + let node = file() + + for action in DiskItemAction.allCases { + let availability = DiskItemActionPolicy.availability( + of: action, + for: node, + isScanRoot: false, + isScanInProgress: false + ) + XCTAssertEqual(availability, .allowed, "\(action) should be offered on a normal row") + } + } + + func testTheUnaccountedPlaceholderOffersNothingAtAll() { + // This row carries the volume root's path, so every action would act on the + // whole volume — a Move to Trash aimed at the mount point. It has to be + // refused before it ever reaches the filesystem. + let placeholder = DiskNode( + path: "/", + name: DiskScanner.unaccountedNodeName, + isDirectory: false, + size: 12_000_000, + children: [], + isSynthetic: true + ) + + for action in DiskItemAction.allCases { + let availability = DiskItemActionPolicy.availability( + of: action, + for: placeholder, + isScanRoot: false, + isScanInProgress: false + ) + XCTAssertFalse(availability.isAllowed, "\(action) must never run against the placeholder") + XCTAssertNotNil(availability.reason) + } + } + + func testTheReconciledPlaceholderIsFlaggedSyntheticByTheScanner() { + // The policy above is only as good as this flag: the placeholder and the + // root are indistinguishable by path. + let child = DiskNode(path: "/Users", name: "Users", isDirectory: true, size: 400, children: []) + let root = DiskNode(path: "/", name: "Macintosh HD", isDirectory: true, size: 400, children: [child]) + + let reconciled = DiskScanner.reconciled(root: root, usedBytes: 1000) + let placeholder = reconciled.children.first { $0.name == DiskScanner.unaccountedNodeName } + + XCTAssertEqual(placeholder?.size, 600) + XCTAssertEqual(placeholder?.path, "/", "the placeholder really does borrow the root's path") + XCTAssertEqual(placeholder?.isSynthetic, true) + XCTAssertEqual(reconciled.isSynthetic, false, "the root itself is a real directory") + XCTAssertEqual(child.isSynthetic, false) + } + + func testTheVolumeBeingAnalyzedCannotBeTrashedButCanStillBeRevealedAndCopied() { + let root = folder(name: "Data", path: "/Volumes/Data") + + XCTAssertFalse( + DiskItemActionPolicy.availability(of: .moveToTrash, for: root, isScanRoot: true, isScanInProgress: false).isAllowed + ) + XCTAssertEqual( + DiskItemActionPolicy.availability(of: .revealInFinder, for: root, isScanRoot: true, isScanInProgress: false), + .allowed + ) + XCTAssertEqual( + DiskItemActionPolicy.availability(of: .copyPath, for: root, isScanRoot: true, isScanInProgress: false), + .allowed + ) + } + + func testTrashingIsHeldBackWhileAScanIsStillWalkingTheVolume() { + // The scan in flight would keep measuring a directory that had moved out + // from under it and then commit a tree built from both halves. + let node = folder() + + let trash = DiskItemActionPolicy.availability(of: .moveToTrash, for: node, isScanRoot: false, isScanInProgress: true) + XCTAssertFalse(trash.isAllowed) + XCTAssertEqual(trash.reason, "A scan is still running on this volume. Let it finish before moving anything to the Trash.") + + // Reading a path is harmless mid-scan, so those two stay available. + XCTAssertEqual( + DiskItemActionPolicy.availability(of: .revealInFinder, for: node, isScanRoot: false, isScanInProgress: true), + .allowed + ) + XCTAssertEqual( + DiskItemActionPolicy.availability(of: .copyPath, for: node, isScanRoot: false, isScanInProgress: true), + .allowed + ) + } + + func testAvailabilityExposesItsReasonOnlyWhenItRefuses() { + XCTAssertNil(DiskItemActionAvailability.allowed.reason) + XCTAssertTrue(DiskItemActionAvailability.allowed.isAllowed) + + let refusal = DiskItemActionAvailability.unavailable(reason: "nope") + XCTAssertEqual(refusal.reason, "nope") + XCTAssertFalse(refusal.isAllowed) + } + + // MARK: - Trashing + + func testTrashingSomethingThatIsNoLongerThereReportsItAsAlreadyGone() { + // The row a user right-clicks can be minutes old; Finder may have removed + // the item in between. That must produce a message, not a crash and not + // silence. + let missing = NSTemporaryDirectory() + "dmonte-disk-analyzer-missing-\(UUID().uuidString)" + + let result = DiskItemTrasher.moveToTrash(path: missing) + + guard case .failure(let failure) = result else { + return XCTFail("a path that does not exist cannot be trashed") + } + XCTAssertEqual(failure, .alreadyGone) + } + + func testAVanishedItemIsRecognisedFromTheThrownErrorToo() { + // The item can also disappear between the existence check and the call, so + // the same condition is classified out of whatever trashItem throws. + XCTAssertEqual( + DiskItemTrasher.failure(from: NSError(domain: NSCocoaErrorDomain, code: NSFileNoSuchFileError)), + .alreadyGone + ) + XCTAssertEqual( + DiskItemTrasher.failure(from: NSError(domain: NSCocoaErrorDomain, code: NSFileReadNoSuchFileError)), + .alreadyGone + ) + XCTAssertEqual( + DiskItemTrasher.failure(from: NSError(domain: NSPOSIXErrorDomain, code: Int(ENOENT))), + .alreadyGone + ) + } + + func testARealRefusalIsPassedThroughVerbatim() { + let denied = NSError( + domain: NSCocoaErrorDomain, + code: NSFileWriteNoPermissionError, + userInfo: [NSLocalizedDescriptionKey: "You don't have permission."] + ) + + XCTAssertEqual(DiskItemTrasher.failure(from: denied), .systemRefused("You don't have permission.")) + } + + func testAPermissionErrorIsNotMistakenForAMissingFile() { + // The two are handled very differently — one prunes the row, the other + // leaves the tree alone — so they must not collapse into each other. + let denied = NSError(domain: NSPOSIXErrorDomain, code: Int(EACCES)) + XCTAssertNotEqual(DiskItemTrasher.failure(from: denied), .alreadyGone) + } + + // MARK: - Existence + + func testTheExistenceProbeSeesARealFileAndNotAMissingOne() throws { + let directory = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("dmonte-disk-probe-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + + let file = directory.appendingPathComponent("present.txt") + try Data("hello".utf8).write(to: file) + + XCTAssertTrue(DiskItemProbe.exists(atPath: file.path)) + XCTAssertTrue(DiskItemProbe.exists(atPath: directory.path)) + XCTAssertFalse(DiskItemProbe.exists(atPath: directory.appendingPathComponent("absent.txt").path)) + XCTAssertFalse(DiskItemProbe.exists(atPath: "")) + } + + func testADanglingSymlinkCountsAsPresentBecauseItIsARealDirectoryEntry() throws { + // FileManager.fileExists follows the link and would call this gone, which + // would leave a row nobody could reveal, copy or trash. + let directory = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("dmonte-disk-probe-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + + let link = directory.appendingPathComponent("dangling") + try FileManager.default.createSymbolicLink( + at: link, + withDestinationURL: directory.appendingPathComponent("never-existed") + ) + + XCTAssertFalse(FileManager.default.fileExists(atPath: link.path)) + XCTAssertTrue(DiskItemProbe.exists(atPath: link.path)) + } + + // MARK: - Wording + + func testTheConfirmationForAFolderSaysEverythingInsideItGoesToo() { + let node = folder(name: "Renders", path: "/Volumes/Data/Renders", size: 1_073_741_824) + + let prompt = DiskItemActionMessage.trashPrompt(for: node) + + XCTAssertEqual(prompt.title, "Move \u{201C}Renders\u{201D} to the Trash?") + XCTAssertTrue(prompt.detail.hasPrefix("/Volumes/Data/Renders"), "the full path is the disambiguator") + XCTAssertTrue(prompt.detail.contains("everything inside it")) + XCTAssertTrue(prompt.detail.contains(UInt64(1_073_741_824).diskBytesString)) + } + + func testTheConfirmationForAFileDoesNotClaimToTakeAnythingWithIt() { + let node = file(name: "clip.mov", path: "/Volumes/Data/clip.mov", size: 5_000_000) + + let prompt = DiskItemActionMessage.trashPrompt(for: node) + + XCTAssertEqual(prompt.title, "Move \u{201C}clip.mov\u{201D} to the Trash?") + XCTAssertFalse(prompt.detail.contains("everything inside it")) + XCTAssertTrue(prompt.detail.contains(UInt64(5_000_000).diskBytesString)) + } + + func testAnAlreadyGoneItemIsExplainedRatherThanReportedAsAFailure() { + let message = DiskItemActionMessage.trashFailed(name: "clip.mov", failure: .alreadyGone) + + XCTAssertTrue(message.contains("already deleted or moved")) + XCTAssertTrue(message.contains("Removed it from the results"), "the row vanishing needs explaining") + } + + func testARefusalQuotesTheSystemReason() { + let message = DiskItemActionMessage.trashFailed(name: "clip.mov", failure: .systemRefused("Volume is read-only.")) + + XCTAssertEqual(message, "Couldn't move clip.mov to the Trash: Volume is read-only.") + } + + func testSuccessNamesTheSpaceThatCameOffTheTotals() { + let message = DiskItemActionMessage.trashSucceeded(name: "Renders", bytes: 1_073_741_824) + + XCTAssertTrue(message.contains("Renders")) + XCTAssertTrue(message.contains(UInt64(1_073_741_824).diskBytesString)) + } + + // MARK: - Copy Path + + func testCopyPathPutsTheFullPathOnTheClipboard() { + // A private pasteboard so the suite never touches the real clipboard. + let pasteboard = NSPasteboard(name: .init("com.havokentity.mactools.tests.disk-\(UUID().uuidString)")) + defer { pasteboard.releaseGlobally() } + + XCTAssertTrue(DiskItemPathCopier.copy(path: "/Volumes/Data/Renders", to: pasteboard)) + XCTAssertEqual(pasteboard.string(forType: .string), "/Volumes/Data/Renders") + } + + func testCopyPathRefusesAnEmptyPathInsteadOfBlankingTheClipboard() { + let pasteboard = NSPasteboard(name: .init("com.havokentity.mactools.tests.disk-\(UUID().uuidString)")) + defer { pasteboard.releaseGlobally() } + + pasteboard.clearContents() + pasteboard.setString("keep me", forType: .string) + + XCTAssertFalse(DiskItemPathCopier.copy(path: "", to: pasteboard)) + XCTAssertEqual(pasteboard.string(forType: .string), "keep me") + } +} diff --git a/Tests/DMonteCoreTests/DiskTreeCacheTests.swift b/Tests/DMonteCoreTests/DiskTreeCacheTests.swift index 762b3b4..41f8c68 100644 --- a/Tests/DMonteCoreTests/DiskTreeCacheTests.swift +++ b/Tests/DMonteCoreTests/DiskTreeCacheTests.swift @@ -37,4 +37,64 @@ final class DiskTreeCacheTests: XCTestCase { XCTAssertNil(cache.path(to: root)) XCTAssertNil(cache.path(to: file)) } + + func testPatchingForATrashedNodeKeepsTheIndexUsableWithoutARebuild() { + // Rebuilding the index walks every node on the volume — seconds on a boot + // disk, during which the tree pane would still be showing the trashed row. + // Patching has to leave an index indistinguishable from a fresh one. + let archive = DiskNode(path: "/root/docs/archive.zip", name: "archive.zip", isDirectory: false, size: 50, children: []) + let notes = DiskNode(path: "/root/docs/notes.txt", name: "notes.txt", isDirectory: false, size: 20, children: []) + let docs = DiskNode(path: "/root/docs", name: "docs", isDirectory: true, size: 70, children: [archive, notes]) + let logs = DiskNode(path: "/root/logs", name: "logs", isDirectory: true, size: 30, children: []) + let root = DiskNode(path: "/root", name: "root", isDirectory: true, size: 100, children: [docs, logs]) + + guard let removal = DiskTreeMutator.removing(archive, from: root) else { + return XCTFail("archive.zip is in the tree, so the removal must succeed") + } + let patched = DiskTreeCache(root: root).applying(removal) + + XCTAssertEqual(patched.nodeCount, 4, "one node left the tree") + XCTAssertTrue(patched.root === removal.root) + XCTAssertNil(patched.path(to: archive), "the trashed row is no longer navigable") + XCTAssertEqual(patched.path(to: removal.root)?.map(\.name), ["root"]) + XCTAssertEqual(patched.path(to: notes)?.map(\.name), ["root", "docs"] + ["notes.txt"]) + XCTAssertEqual(patched.path(to: notes)?.map(\.size), [50, 20, 20], "the walk goes through the resized ancestors") + XCTAssertEqual(patched.path(to: logs)?.map(\.name), ["root", "logs"], "an untouched branch still resolves") + } + + func testPatchingForATrashedDirectoryForgetsItsWholeSubtree() { + let archive = DiskNode(path: "/root/docs/archive.zip", name: "archive.zip", isDirectory: false, size: 50, children: []) + let docs = DiskNode(path: "/root/docs", name: "docs", isDirectory: true, size: 50, children: [archive]) + let logs = DiskNode(path: "/root/logs", name: "logs", isDirectory: true, size: 30, children: []) + let root = DiskNode(path: "/root", name: "root", isDirectory: true, size: 80, children: [docs, logs]) + + guard let removal = DiskTreeMutator.removing(docs, from: root) else { + return XCTFail("docs is in the tree, so the removal must succeed") + } + let patched = DiskTreeCache(root: root).applying(removal) + + XCTAssertEqual(patched.nodeCount, 2, "the folder and its child both left") + XCTAssertNil(patched.path(to: docs)) + XCTAssertNil(patched.path(to: archive), "a descendant of a trashed folder must not stay reachable") + XCTAssertEqual(patched.path(to: logs)?.map(\.size), [30, 30]) + } + + func testAPatchedIndexMatchesOneBuiltFromScratch() { + let deep = DiskNode(path: "/root/a/b/leaf", name: "leaf", isDirectory: false, size: 10, children: []) + let sibling = DiskNode(path: "/root/a/b/other", name: "other", isDirectory: false, size: 5, children: []) + let b = DiskNode(path: "/root/a/b", name: "b", isDirectory: true, size: 15, children: [deep, sibling]) + let a = DiskNode(path: "/root/a", name: "a", isDirectory: true, size: 15, children: [b]) + let root = DiskNode(path: "/root", name: "root", isDirectory: true, size: 15, children: [a]) + + guard let removal = DiskTreeMutator.removing(deep, from: root) else { + return XCTFail("leaf is in the tree, so the removal must succeed") + } + let patched = DiskTreeCache(root: root).applying(removal) + let rebuilt = DiskTreeCache(root: removal.root) + + XCTAssertEqual(patched.nodeCount, rebuilt.nodeCount) + XCTAssertEqual(patched.path(to: sibling)?.map(\.name), rebuilt.path(to: sibling)?.map(\.name)) + XCTAssertEqual(patched.path(to: sibling)?.map(\.size), rebuilt.path(to: sibling)?.map(\.size)) + XCTAssertEqual(patched.path(to: removal.root)?.count, rebuilt.path(to: removal.root)?.count) + } } diff --git a/Tests/DMonteCoreTests/DiskTreeMutationTests.swift b/Tests/DMonteCoreTests/DiskTreeMutationTests.swift new file mode 100644 index 0000000..daf6382 --- /dev/null +++ b/Tests/DMonteCoreTests/DiskTreeMutationTests.swift @@ -0,0 +1,290 @@ +import XCTest +@testable import DMonteCore + +/// The arithmetic behind "this is no longer on disk, so stop counting it". +/// +/// Every one of these assertions is about a total the user reads off the screen and +/// acts on: if a parent's size does not shrink by exactly what left it, the analyzer +/// is telling people to delete space they have already freed. +final class DiskTreeMutationTests: XCTestCase { + /// root (100) + /// docs (70) + /// archive.zip (50) + /// notes.txt (20) + /// logs (30) + /// today.log (30) + private struct Fixture { + let archive: DiskNode + let notes: DiskNode + let docs: DiskNode + let todayLog: DiskNode + let logs: DiskNode + let root: DiskNode + + init() { + archive = DiskNode(path: "/root/docs/archive.zip", name: "archive.zip", isDirectory: false, size: 50, children: []) + notes = DiskNode(path: "/root/docs/notes.txt", name: "notes.txt", isDirectory: false, size: 20, children: []) + docs = DiskNode(path: "/root/docs", name: "docs", isDirectory: true, size: 70, children: [archive, notes]) + todayLog = DiskNode(path: "/root/logs/today.log", name: "today.log", isDirectory: false, size: 30, children: []) + logs = DiskNode(path: "/root/logs", name: "logs", isDirectory: true, size: 30, children: [todayLog]) + root = DiskNode(path: "/root", name: "root", isDirectory: true, size: 100, children: [docs, logs]) + } + } + + // MARK: - Removing a leaf + + func testRemovingALeafShrinksEveryAncestorByExactlyItsSize() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.notes, from: tree.root) else { + return XCTFail("notes.txt is in the tree, so the removal must succeed") + } + + XCTAssertEqual(removal.root.size, 80, "the root loses the 20 bytes that left it") + XCTAssertEqual(removal.replacedAncestors.map(\.new.name), ["root", "docs"], "ancestors come back root first") + + let newDocs = removal.root.children.first { $0.name == "docs" } + XCTAssertEqual(newDocs?.size, 50) + XCTAssertEqual(newDocs?.children.map(\.name), ["archive.zip"], "the removed row is gone from its parent") + } + + func testRemovingALeafLeavesUntouchedSubtreesIdentical() { + // Only the chain down to the removed node is rebuilt. Everything else is + // carried over by reference, which is what lets the tree index be patched + // instead of rebuilt and what keeps the expanded-rows set meaningful. + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.notes, from: tree.root) else { + return XCTFail("notes.txt is in the tree, so the removal must succeed") + } + + let newLogs = removal.root.children.first { $0.name == "logs" } + XCTAssertTrue(newLogs === tree.logs, "an unaffected sibling keeps its identity") + + let newDocs = removal.root.children.first { $0.name == "docs" } + XCTAssertTrue(newDocs?.children.first === tree.archive, "a surviving child keeps its identity") + XCTAssertFalse(newDocs === tree.docs, "the resized ancestor is a new object") + } + + // MARK: - Removing a whole subtree + + func testRemovingADirectoryTakesItsWholeSubtreeOutOfTheTotals() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.docs, from: tree.root) else { + return XCTFail("docs is in the tree, so the removal must succeed") + } + + XCTAssertEqual(removal.root.size, 30, "the root loses the folder's entire 70 bytes, not just its own") + XCTAssertEqual(removal.root.children.map(\.name), ["logs"]) + XCTAssertEqual(removal.replacedAncestors.map(\.new.name), ["root"], "only the root sat above docs") + } + + func testRemovingADirectoryLeavesNoneOfItsDescendantsReachable() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.docs, from: tree.root) else { + return XCTFail("docs is in the tree, so the removal must succeed") + } + + XCTAssertNil(DiskTreeMutator.ancestorPath(to: tree.archive, from: removal.root)) + XCTAssertNil(DiskTreeMutator.ancestorPath(to: tree.notes, from: removal.root)) + XCTAssertNil(DiskTreeMutator.ancestorPath(to: tree.docs, from: removal.root)) + XCTAssertNotNil(DiskTreeMutator.ancestorPath(to: tree.todayLog, from: removal.root)) + } + + // MARK: - Removing the largest child + + func testRemovingTheLargestChildResortsTheDirectoriesItShrank() { + // docs (70) outranks logs (30) only because of archive.zip. Take that away + // and docs is the smaller of the two, so the biggest-first order the treemap + // and the tree pane both read has to be restored. + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.archive, from: tree.root) else { + return XCTFail("archive.zip is in the tree, so the removal must succeed") + } + + XCTAssertEqual(removal.root.size, 50) + XCTAssertEqual(removal.root.children.map(\.name), ["logs", "docs"], "the shrunken folder drops below its sibling") + XCTAssertEqual(removal.root.children.map(\.size), [30, 20]) + + let newDocs = removal.root.children.first { $0.name == "docs" } + XCTAssertEqual(newDocs?.size, 20) + XCTAssertEqual(newDocs?.children.map(\.name), ["notes.txt"]) + } + + func testRemovingTheLargestChildOfTheRootKeepsTheRestOfTheTreeExact() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.docs, from: tree.root) else { + return XCTFail("docs is in the tree, so the removal must succeed") + } + + let sum = removal.root.children.reduce(UInt64(0)) { $0 + $1.size } + XCTAssertEqual(sum, removal.root.size, "the root still equals the sum of what is left under it") + } + + // MARK: - Refusals + + func testRemovingTheRootIsRefused() { + let tree = Fixture() + XCTAssertNil(DiskTreeMutator.removing(tree.root, from: tree.root), "a scan has to keep its root") + } + + func testRemovingANodeFromAnotherTreeIsRefused() { + let tree = Fixture() + let stranger = DiskNode(path: "/elsewhere/file", name: "file", isDirectory: false, size: 5, children: []) + + XCTAssertNil(DiskTreeMutator.removing(stranger, from: tree.root)) + } + + func testRemovingMatchesByIdentityNotByPath() { + // The reconciliation placeholder deliberately carries the volume root's + // path, so path-string matching would let a Trash aimed at the placeholder + // land on the root instead. + let placeholder = DiskNode( + path: "/root", + name: DiskScanner.unaccountedNodeName, + isDirectory: false, + size: 25, + children: [], + isSynthetic: true + ) + let real = DiskNode(path: "/root/big.bin", name: "big.bin", isDirectory: false, size: 75, children: []) + let root = DiskNode(path: "/root", name: "root", isDirectory: true, size: 100, children: [real, placeholder]) + + guard let removal = DiskTreeMutator.removing(placeholder, from: root) else { + return XCTFail("the placeholder is a child of the root, so it is removable from the tree") + } + + XCTAssertEqual(removal.root.children.map(\.name), ["big.bin"], "the identical path must not confuse the search") + XCTAssertEqual(removal.root.size, 75) + } + + func testAnAncestorSmallerThanTheNodeItLosesClampsAtZeroInsteadOfWrapping() { + // Sizes are UInt64, so a tree that disagrees with itself would otherwise + // underflow into an 18-exabyte folder on screen. + let child = DiskNode(path: "/root/huge", name: "huge", isDirectory: false, size: 500, children: []) + let root = DiskNode(path: "/root", name: "root", isDirectory: true, size: 100, children: [child]) + + guard let removal = DiskTreeMutator.removing(child, from: root) else { + return XCTFail("huge is a child of the root, so the removal must succeed") + } + + XCTAssertEqual(removal.root.size, 0) + } + + func testDeepChainShrinksEveryLevel() { + let leaf = DiskNode(path: "/a/b/c/leaf", name: "leaf", isDirectory: false, size: 40, children: []) + let c = DiskNode(path: "/a/b/c", name: "c", isDirectory: true, size: 40, children: [leaf]) + let b = DiskNode(path: "/a/b", name: "b", isDirectory: true, size: 60, children: [c]) + let a = DiskNode(path: "/a", name: "a", isDirectory: true, size: 90, children: [b]) + + guard let removal = DiskTreeMutator.removing(leaf, from: a) else { + return XCTFail("leaf is in the tree, so the removal must succeed") + } + + XCTAssertEqual(removal.replacedAncestors.map(\.new.name), ["a", "b", "c"]) + XCTAssertEqual(removal.replacedAncestors.map(\.new.size), [50, 20, 0]) + XCTAssertEqual(removal.root.size, 50) + } + + // MARK: - Navigation state + + func testTrashingTheFolderYouAreInsideWalksBackToItsParent() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.docs, from: tree.root) else { + return XCTFail("docs is in the tree, so the removal must succeed") + } + + let patched = DiskTreeNavigationPatch.patchedPath( + [tree.root, tree.docs], + removing: removal.removed, + using: DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors), + fallbackRoot: removal.root + ) + + XCTAssertEqual(patched.map(\.name), ["root"]) + XCTAssertTrue(patched.first === removal.root, "the breadcrumb points at the resized root, not the stale one") + } + + func testTrashingElsewhereKeepsTheBreadcrumbButSwapsInTheResizedAncestors() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.notes, from: tree.root) else { + return XCTFail("notes.txt is in the tree, so the removal must succeed") + } + + let patched = DiskTreeNavigationPatch.patchedPath( + [tree.root, tree.docs], + removing: removal.removed, + using: DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors), + fallbackRoot: removal.root + ) + + XCTAssertEqual(patched.map(\.name), ["root", "docs"]) + XCTAssertEqual(patched.map(\.size), [80, 50], "the breadcrumb shows the new totals, not the ones from the scan") + XCTAssertFalse(patched.last === tree.docs) + } + + func testAnEmptiedBreadcrumbFallsBackToTheRoot() { + // Nothing may leave the stack empty: the treemap draws from its last entry. + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.docs, from: tree.root) else { + return XCTFail("docs is in the tree, so the removal must succeed") + } + + let patched = DiskTreeNavigationPatch.patchedPath( + [tree.docs], + removing: removal.removed, + using: [:], + fallbackRoot: removal.root + ) + + XCTAssertEqual(patched.count, 1) + XCTAssertTrue(patched.first === removal.root) + } + + func testExpandedFoldersStayExpandedAcrossTheRebuild() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.notes, from: tree.root) else { + return XCTFail("notes.txt is in the tree, so the removal must succeed") + } + + let patched = DiskTreeNavigationPatch.patchedExpansion( + [tree.root.id, tree.docs.id, tree.logs.id], + using: removal.replacedAncestors + ) + + XCTAssertFalse(patched.contains(tree.root.id), "the stale ancestor id is dropped") + XCTAssertFalse(patched.contains(tree.docs.id)) + XCTAssertTrue(patched.contains(removal.root.id)) + XCTAssertTrue(patched.contains(removal.replacedAncestors[1].new.id)) + XCTAssertTrue(patched.contains(tree.logs.id), "an untouched folder is left alone") + } + + func testHighlightFollowsAResizedAncestorAndClearsForTheTrashedRow() { + let tree = Fixture() + + guard let removal = DiskTreeMutator.removing(tree.notes, from: tree.root) else { + return XCTFail("notes.txt is in the tree, so the removal must succeed") + } + let replacements = DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors) + + XCTAssertNil( + DiskTreeNavigationPatch.patchedSelection(tree.notes, removing: removal.removed, using: replacements), + "the row that was trashed cannot stay selected" + ) + XCTAssertTrue( + DiskTreeNavigationPatch.patchedSelection(tree.docs, removing: removal.removed, using: replacements) === removal.replacedAncestors[1].new + ) + XCTAssertTrue( + DiskTreeNavigationPatch.patchedSelection(tree.logs, removing: removal.removed, using: replacements) === tree.logs + ) + XCTAssertNil(DiskTreeNavigationPatch.patchedSelection(nil, removing: removal.removed, using: replacements)) + } +} From da36e636c3e19a77e399a240673bd008a6621acf Mon Sep 17 00:00:00 2001 From: Mac Studio M4MAX DMonte Date: Tue, 21 Jul 2026 23:09:28 +0530 Subject: [PATCH 2/2] Disk Analyzer: stop a trashed folder leaving its contents selected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trashing a folder pruned it out of the tree index but left any selection that lived inside it exactly where it was. patchedSelection only compared the selection against the removed node itself, so a file the user had clicked one level down was neither that node nor one of the rebuilt ancestors and came back unchanged — pointing into a subtree the index had already forgotten. The action bar stayed enabled on it, the hover chip kept drawing its name and size, and Cmd-Delete raised a real "Move to the Trash?" confirmation for a file that was already in the Trash. Confirming reported "Removed it from the results" while pruning nothing, and if anything had since been written to that path, the confirmed shortcut would have trashed the new occupant instead. The selection patch now asks the post-prune index whether the row it is about to keep is still in the tree, which is the only thing that knows a descendant left with its parent. Walking the removed subtree would answer the same question at a cost proportional to the folder that was just trashed, which is the wrong price for a check that runs on every prune. Two smaller things the same review turned up. focusedNode is what Cmd-Delete acts on, but nothing drew it: clicking a treemap tile focused it and cleared the hover, and Back or a breadcrumb jump then left a destructive shortcut armed at a folder two levels away with nothing on screen saying so. Treemap tiles and tree rows now outline the focused row, and goUp/navigate clear it alongside the hover the way every other navigation reset already does. And a second Move to Trash attempted while one was still in flight was refused with the scan sentence, telling the user to wait for a scan that was not running and that the progress UI never showed; the policy now has its own case for it. Co-Authored-By: Claude Opus 4.8 --- Sources/DMonteCore/DiskAnalyzer.swift | 66 ++++++- Sources/DMonteCore/DiskItemActions.swift | 11 +- Sources/DMonteCore/DiskTreeMutation.swift | 19 +- .../DiskItemActionsTests.swift | 53 ++++++ .../DiskTreeMutationTests.swift | 162 +++++++++++++++++- 5 files changed, 293 insertions(+), 18 deletions(-) diff --git a/Sources/DMonteCore/DiskAnalyzer.swift b/Sources/DMonteCore/DiskAnalyzer.swift index 515081c..606514d 100644 --- a/Sources/DMonteCore/DiskAnalyzer.swift +++ b/Sources/DMonteCore/DiskAnalyzer.swift @@ -585,6 +585,14 @@ struct DiskTreeCache { self.indexedNodeIDs = indexedNodeIDs } + /// Whether this node is still part of the indexed tree. Cheap enough to ask after + /// every prune, which is what lets the navigation patch drop a highlight that the + /// removal made unreachable instead of leaving it pointing into a forgotten + /// subtree. + func contains(_ node: DiskNode) -> Bool { + indexedNodeIDs.contains(node.id) + } + func path(to node: DiskNode) -> [DiskNode]? { guard indexedNodeIDs.contains(node.id) else { return nil } @@ -1297,6 +1305,7 @@ public struct DiskAnalyzerWindowView: View { TreemapTile( entry: entry, isHovered: hoveredNode?.id == entry.node.id, + isFocused: focusedNode?.id == entry.node.id, layout: layout ) .frame(width: max(0, entry.rect.width), height: max(0, entry.rect.height)) @@ -1386,6 +1395,7 @@ public struct DiskAnalyzerWindowView: View { expandedNodeIDs: expandedTreeNodeIDs, currentNodeID: currentNode?.id, highlightedNodeID: hoveredNode?.id, + focusedNodeID: focusedNode?.id, layout: layout, onToggleExpand: toggleTreeExpansion, onSelect: navigateFromTree, @@ -1823,13 +1833,23 @@ public struct DiskAnalyzerWindowView: View { private func goUp() { guard pathStack.count > 1 else { return } pathStack.removeLast() - hoveredNode = nil + clearSelection() } private func navigate(toIndex index: Int) { guard index < pathStack.count else { return } pathStack = Array(pathStack.prefix(index + 1)) + clearSelection() + } + + /// Drops both highlights when the view moves somewhere the user did not pick a + /// row. `focusedNode` goes with `hoveredNode` because it is what ⌘⌫ acts on: a + /// selection left behind by a breadcrumb jump is off screen and usually two + /// levels away, and a destructive shortcut must never aim at something the + /// window is not showing as picked. + private func clearSelection() { hoveredNode = nil + focusedNode = nil } /// True while nothing on the selected volume can safely be trashed: either the @@ -1845,7 +1865,8 @@ public struct DiskAnalyzerWindowView: View { of: action, for: node, isScanRoot: selectedTreeCache?.root.id == node.id || pathStack.first?.id == node.id, - isScanInProgress: isTreeUnsettled || isTrashInFlight + isScanInProgress: isTreeUnsettled, + isTrashInFlight: isTrashInFlight ) } @@ -1969,7 +1990,8 @@ public struct DiskAnalyzerWindowView: View { // Patching the index instead of rebuilding it keeps the tree pane populated; // a rebuild walks every node on the volume and would show the trashed row // for the whole of that walk. - treeCaches[volumeId] = cache.applying(removal) + let prunedCache = cache.applying(removal) + treeCaches[volumeId] = prunedCache let replacements = DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors) expandedTreeNodeIDs = DiskTreeNavigationPatch.patchedExpansion( @@ -1987,8 +2009,20 @@ public struct DiskAnalyzerWindowView: View { using: replacements, fallbackRoot: removal.root ) - hoveredNode = DiskTreeNavigationPatch.patchedSelection(hoveredNode, removing: removal.removed, using: replacements) - focusedNode = DiskTreeNavigationPatch.patchedSelection(focusedNode, removing: removal.removed, using: replacements) + // The pruned index, not the old one: a row that lived inside the trashed + // folder went with it, and only the post-prune index knows that. + hoveredNode = DiskTreeNavigationPatch.patchedSelection( + hoveredNode, + removing: removal.removed, + using: replacements, + survivesRemoval: prunedCache.contains + ) + focusedNode = DiskTreeNavigationPatch.patchedSelection( + focusedNode, + removing: removal.removed, + using: replacements, + survivesRemoval: prunedCache.contains + ) } // Treemap layouts are keyed by node identity, so the rebuilt spine simply @@ -2091,6 +2125,10 @@ private struct DiskTreeRow: View { var expandedNodeIDs: Set var currentNodeID: ObjectIdentifier? var highlightedNodeID: ObjectIdentifier? + /// The row the action bar and its shortcuts would act on. Outlined rather than + /// tinted, so it still reads as the selection on the row that is also the folder + /// the treemap is showing — and so ⌘⌫ always has a visible target. + var focusedNodeID: ObjectIdentifier? var layout: DiskAnalyzerLayout var onToggleExpand: (DiskNode) -> Void var onSelect: (DiskNode) -> Void @@ -2154,6 +2192,12 @@ private struct DiskTreeRow: View { .padding(.trailing, 8) .frame(height: layout.treeRowHeight) .background(rowBackground) + .overlay { + if isFocused { + RoundedRectangle(cornerRadius: 4, style: .continuous) + .strokeBorder(Color.white.opacity(0.55), style: StrokeStyle(lineWidth: 1, dash: [3, 2])) + } + } if isExpanded { ForEach(node.children) { child in @@ -2163,6 +2207,7 @@ private struct DiskTreeRow: View { expandedNodeIDs: expandedNodeIDs, currentNodeID: currentNodeID, highlightedNodeID: highlightedNodeID, + focusedNodeID: focusedNodeID, layout: layout, onToggleExpand: onToggleExpand, onSelect: onSelect, @@ -2195,6 +2240,10 @@ private struct DiskTreeRow: View { highlightedNodeID == node.id } + private var isFocused: Bool { + focusedNodeID == node.id + } + private var rowBackground: some ShapeStyle { if isCurrent { return Color.accentColor.opacity(0.20) @@ -2209,6 +2258,10 @@ private struct DiskTreeRow: View { private struct TreemapTile: View { var entry: TreemapRect var isHovered: Bool + /// The tile the row actions and their shortcuts would act on. Drawn separately + /// from the hover ring because the pointer moves on but the selection does not, + /// and ⌘⌫ has to have a visible target before the confirmation appears. + var isFocused: Bool var layout: DiskAnalyzerLayout var body: some View { @@ -2251,6 +2304,9 @@ private struct TreemapTile: View { if isHovered { Rectangle() .strokeBorder(Color.white, lineWidth: 1.5) + } else if isFocused { + Rectangle() + .strokeBorder(Color.white.opacity(0.75), style: StrokeStyle(lineWidth: 1.5, dash: [3, 2])) } } .contentShape(Rectangle()) diff --git a/Sources/DMonteCore/DiskItemActions.swift b/Sources/DMonteCore/DiskItemActions.swift index 5a6b2fc..8282dcf 100644 --- a/Sources/DMonteCore/DiskItemActions.swift +++ b/Sources/DMonteCore/DiskItemActions.swift @@ -37,7 +37,8 @@ public enum DiskItemActionPolicy { of action: DiskItemAction, for node: DiskNode, isScanRoot: Bool, - isScanInProgress: Bool + isScanInProgress: Bool, + isTrashInFlight: Bool = false ) -> DiskItemActionAvailability { // The "Unaccounted / Inaccessible" node carries the volume root's path so // the treemap can place it, but it stands for bytes macOS reports that we @@ -70,6 +71,14 @@ public enum DiskItemActionPolicy { reason: "A scan is still running on this volume. Let it finish before moving anything to the Trash." ) } + if isTrashInFlight { + // Distinct from the scan case on purpose. Trashing a large folder onto + // a slow volume takes seconds, and telling the user a scan is running + // when none is leaves them waiting on something the window never shows. + return .unavailable( + reason: "A Move to Trash is still finishing. Wait for it to complete before starting another one." + ) + } return .allowed } } diff --git a/Sources/DMonteCore/DiskTreeMutation.swift b/Sources/DMonteCore/DiskTreeMutation.swift index 9405044..c107c7a 100644 --- a/Sources/DMonteCore/DiskTreeMutation.swift +++ b/Sources/DMonteCore/DiskTreeMutation.swift @@ -176,15 +176,26 @@ enum DiskTreeNavigationPatch { return patched } - /// The hovered/focused row after the rebuild: gone if it was the node we removed, - /// otherwise remapped so the highlight follows a rebuilt ancestor. + /// The hovered/focused row after the rebuild: gone if the removal put it out of + /// the tree, otherwise remapped so the highlight follows a rebuilt ancestor. + /// + /// Testing identity against `removed` alone is not enough, because everything + /// *underneath* the removed node left the tree with it. A descendant left in + /// place would keep the action bar enabled and the hover chip drawn for a row the + /// tree has already forgotten, and ⌘⌫ would then raise a real "Move to the Trash?" + /// confirmation for a file that is already in the Trash — or, worse, for whatever + /// has since been written to that path. `survivesRemoval` is how the caller's + /// index answers "is this still in the tree?" in constant time; walking the + /// removed subtree to find out would cost as much as the trashed folder is big. static func patchedSelection( _ selection: DiskNode?, removing removed: DiskNode, - using replacements: [ObjectIdentifier: DiskNode] + using replacements: [ObjectIdentifier: DiskNode], + survivesRemoval: (DiskNode) -> Bool ) -> DiskNode? { guard let selection else { return nil } if selection === removed { return nil } - return replacements[selection.id] ?? selection + let patched = replacements[selection.id] ?? selection + return survivesRemoval(patched) ? patched : nil } } diff --git a/Tests/DMonteCoreTests/DiskItemActionsTests.swift b/Tests/DMonteCoreTests/DiskItemActionsTests.swift index 8c6584b..2d526da 100644 --- a/Tests/DMonteCoreTests/DiskItemActionsTests.swift +++ b/Tests/DMonteCoreTests/DiskItemActionsTests.swift @@ -104,6 +104,59 @@ final class DiskItemActionsTests: XCTestCase { ) } + func testASecondTrashWhileOneIsStillFinishingSaysSoInsteadOfBlamingAScan() { + // Trashing a large folder onto a slow external volume takes seconds, and the + // latch that refuses a second one during that window used to borrow the scan + // sentence — telling the user to wait for a scan that is not running and that + // the progress UI never shows. + let node = folder() + + let trash = DiskItemActionPolicy.availability( + of: .moveToTrash, + for: node, + isScanRoot: false, + isScanInProgress: false, + isTrashInFlight: true + ) + XCTAssertFalse(trash.isAllowed) + XCTAssertEqual( + trash.reason, + "A Move to Trash is still finishing. Wait for it to complete before starting another one." + ) + XCTAssertNotEqual( + trash.reason, + "A scan is still running on this volume. Let it finish before moving anything to the Trash.", + "the two waits are different things to wait for and must not share a sentence" + ) + + // The latch is only about writing, so the read-only pair stay available. + for action in [DiskItemAction.revealInFinder, .copyPath] { + XCTAssertEqual( + DiskItemActionPolicy.availability( + of: action, + for: node, + isScanRoot: false, + isScanInProgress: false, + isTrashInFlight: true + ), + .allowed + ) + } + } + + func testAnIdleTrashLatchDoesNotHoldBackTrashing() { + XCTAssertEqual( + DiskItemActionPolicy.availability( + of: .moveToTrash, + for: folder(), + isScanRoot: false, + isScanInProgress: false, + isTrashInFlight: false + ), + .allowed + ) + } + func testAvailabilityExposesItsReasonOnlyWhenItRefuses() { XCTAssertNil(DiskItemActionAvailability.allowed.reason) XCTAssertTrue(DiskItemActionAvailability.allowed.isAllowed) diff --git a/Tests/DMonteCoreTests/DiskTreeMutationTests.swift b/Tests/DMonteCoreTests/DiskTreeMutationTests.swift index daf6382..ca79215 100644 --- a/Tests/DMonteCoreTests/DiskTreeMutationTests.swift +++ b/Tests/DMonteCoreTests/DiskTreeMutationTests.swift @@ -267,24 +267,170 @@ final class DiskTreeMutationTests: XCTestCase { XCTAssertTrue(patched.contains(tree.logs.id), "an untouched folder is left alone") } + /// Prunes `node` out of `tree` exactly the way the view does — through the index — + /// and hands back everything `patchedSelection` needs. Going through a real + /// `DiskTreeCache` matters: the whole point of these assertions is that the pruned + /// index is the only thing that knows a descendant left the tree. + private func prune( + _ node: DiskNode, + in tree: Fixture, + file: StaticString = #filePath, + line: UInt = #line + ) -> (removal: DiskTreeRemoval, replacements: [ObjectIdentifier: DiskNode], survives: (DiskNode) -> Bool)? { + let cache = DiskTreeCache(root: tree.root) + guard let path = cache.path(to: node), let removal = DiskTreeMutator.removing(nodeAt: path) else { + XCTFail("\(node.name) is in the tree, so the removal must succeed", file: file, line: line) + return nil + } + let pruned = cache.applying(removal) + return (removal, DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors), pruned.contains) + } + func testHighlightFollowsAResizedAncestorAndClearsForTheTrashedRow() { let tree = Fixture() - guard let removal = DiskTreeMutator.removing(tree.notes, from: tree.root) else { - return XCTFail("notes.txt is in the tree, so the removal must succeed") - } - let replacements = DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors) + guard let (removal, replacements, survives) = prune(tree.notes, in: tree) else { return } XCTAssertNil( - DiskTreeNavigationPatch.patchedSelection(tree.notes, removing: removal.removed, using: replacements), + DiskTreeNavigationPatch.patchedSelection( + tree.notes, + removing: removal.removed, + using: replacements, + survivesRemoval: survives + ), "the row that was trashed cannot stay selected" ) XCTAssertTrue( - DiskTreeNavigationPatch.patchedSelection(tree.docs, removing: removal.removed, using: replacements) === removal.replacedAncestors[1].new + DiskTreeNavigationPatch.patchedSelection( + tree.docs, + removing: removal.removed, + using: replacements, + survivesRemoval: survives + ) === removal.replacedAncestors[1].new + ) + XCTAssertTrue( + DiskTreeNavigationPatch.patchedSelection( + tree.logs, + removing: removal.removed, + using: replacements, + survivesRemoval: survives + ) === tree.logs + ) + XCTAssertNil( + DiskTreeNavigationPatch.patchedSelection( + nil, + removing: removal.removed, + using: replacements, + survivesRemoval: survives + ) + ) + } + + func testTrashingAFolderClearsAHighlightOnSomethingInsideIt() { + // The regression: click archive.zip in the tree pane, then trash its parent + // folder docs. archive.zip is not === the removed node and is not one of the + // rebuilt ancestors, so a selection patch that only checks those two leaves it + // armed — the action bar stays enabled and ⌘⌫ raises a real "Move to the + // Trash?" confirmation for a file that is already in the Trash, or for whatever + // has since been written to that path. + let tree = Fixture() + + guard let (removal, replacements, survives) = prune(tree.docs, in: tree) else { return } + + XCTAssertNil( + DiskTreeNavigationPatch.patchedSelection( + tree.archive, + removing: removal.removed, + using: replacements, + survivesRemoval: survives + ), + "a row that went to the Trash inside the folder cannot stay armed for ⌘⌫" + ) + XCTAssertNil( + DiskTreeNavigationPatch.patchedSelection( + tree.notes, + removing: removal.removed, + using: replacements, + survivesRemoval: survives + ), + "every child of the trashed folder is equally unreachable" + ) + XCTAssertTrue( + DiskTreeNavigationPatch.patchedSelection( + tree.todayLog, + removing: removal.removed, + using: replacements, + survivesRemoval: survives + ) === tree.todayLog, + "a row in an untouched subtree keeps both its identity and its selection" + ) + } + + func testTrashingAFolderClearsAHighlightOnAGrandchildOfIt() { + // Depth is what makes this worth its own case: the removed node's immediate + // children are at least reachable by looking one level down, but a selection + // several levels inside is only detectable through the index. + let shot = DiskNode(path: "/vol/Renders/scene/shot.mov", name: "shot.mov", isDirectory: false, size: 40, children: []) + let scene = DiskNode(path: "/vol/Renders/scene", name: "scene", isDirectory: true, size: 40, children: [shot]) + let renders = DiskNode(path: "/vol/Renders", name: "Renders", isDirectory: true, size: 40, children: [scene]) + let keep = DiskNode(path: "/vol/keep.txt", name: "keep.txt", isDirectory: false, size: 10, children: []) + let root = DiskNode(path: "/vol", name: "vol", isDirectory: true, size: 50, children: [renders, keep]) + + let cache = DiskTreeCache(root: root) + guard let path = cache.path(to: renders), let removal = DiskTreeMutator.removing(nodeAt: path) else { + return XCTFail("Renders is in the tree, so the removal must succeed") + } + let pruned = cache.applying(removal) + let replacements = DiskTreeNavigationPatch.replacementMap(for: removal.replacedAncestors) + + for orphan in [scene, shot] { + XCTAssertNil( + DiskTreeNavigationPatch.patchedSelection( + orphan, + removing: removal.removed, + using: replacements, + survivesRemoval: pruned.contains + ), + "\(orphan.name) left the tree with Renders, so the selection has to go with it" + ) + } + + XCTAssertTrue( + DiskTreeNavigationPatch.patchedSelection( + keep, + removing: removal.removed, + using: replacements, + survivesRemoval: pruned.contains + ) === keep + ) + } + + func testAHighlightSurvivesOnlyWhileTheIndexStillHoldsIt() { + // patchedSelection asks the caller's index rather than walking the removed + // subtree, so pin the contract directly: whatever the index disowns is dropped, + // and a rebuilt ancestor is checked in its rebuilt form, not its stale one. + let tree = Fixture() + + guard let (removal, replacements, _) = prune(tree.notes, in: tree) else { return } + let rebuiltDocs = removal.replacedAncestors[1].new + + XCTAssertNil( + DiskTreeNavigationPatch.patchedSelection( + tree.docs, + removing: removal.removed, + using: replacements, + survivesRemoval: { _ in false } + ), + "an index that holds nothing leaves nothing selected" ) XCTAssertTrue( - DiskTreeNavigationPatch.patchedSelection(tree.logs, removing: removal.removed, using: replacements) === tree.logs + DiskTreeNavigationPatch.patchedSelection( + tree.docs, + removing: removal.removed, + using: replacements, + survivesRemoval: { $0 === rebuiltDocs } + ) === rebuiltDocs, + "the remap happens before the membership test, so the rebuilt node is what gets checked" ) - XCTAssertNil(DiskTreeNavigationPatch.patchedSelection(nil, removing: removal.removed, using: replacements)) } }