Disk Analyzer: act on what the scan finds, and keep the totals true - #28
Open
havokentity wants to merge 2 commits into
Open
Disk Analyzer: act on what the scan finds, and keep the totals true#28havokentity wants to merge 2 commits into
havokentity wants to merge 2 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 2 Phase A.
Implementation: Disk Analyzer: act on what the scan finds, and keep the totals true
Review fixes: Disk Analyzer: stop a trashed folder leaving its contents selected
Built by an isolated agent, then adversarially reviewed by an independent agent that checked the branch out and ran
swift build+ the full suite itself. Every review finding was fixed, with a regression test added for each high and medium.🤖 Generated with Claude Code