macos: make diff sidebar button a visual toggle#7
Merged
Conversation
Author
|
bvolpato-dd
force-pushed
the
bv/code-changes-toggle
branch
from
22ddf96 to
35e2836
Compare
The plusminus titlebar button now uses pushOnPushOff so it appears selected when the diff sidebar is open and unselected when closed. Co-authored-by: Cursor <cursoragent@cursor.com>
bvolpato-dd
force-pushed
the
bv/code-changes-toggle
branch
from
35e2836 to
29d0725
Compare
nicosuave
pushed a commit
that referenced
this pull request
Feb 26, 2026
This PR introduces unit tests and a supporting Mock NSView for testing the SplitTree implementation in Swift. It includes 51 tests which achieve approximately 93.13% (949/1019) coverage of SplitTree.swift's branches. <details> <summary>Coverage</summary> <pre> ./ghostty/macos/Sources/Features/Splits/SplitTree.swift 93.13% (949/1019) SplitTree.Path.isEmpty.getter 100.00% (1/1) SplitTree.isEmpty.getter 100.00% (3/3) SplitTree.isSplit.getter 100.00% (3/3) SplitTree.init() 100.00% (3/3) SplitTree.init(view:) 100.00% (3/3) SplitTree.contains(_:) 100.00% (4/4) SplitTree.inserting(view:at:direction:) 100.00% (6/6) SplitTree.find(id:) 100.00% (4/4) SplitTree.removing(_:) 93.75% (15/16) SplitTree.replacing(node:with:) 93.75% (15/16) SplitTree.focusTarget(for:from:) 82.14% (46/56) closure #1 in SplitTree.focusTarget(for:from:) 100.00% (1/1) closure #2 in SplitTree.focusTarget(for:from:) 100.00% (1/1) closure #3 in SplitTree.focusTarget(for:from:) 100.00% (3/3) implicit closure #1 in SplitTree.focusTarget(for:from:) 0.00% (0/1) SplitTree.equalized() 100.00% (5/5) SplitTree.resizing(node:by:in:with:) 92.00% (69/75) closure #1 in SplitTree.resizing(node:by:in:with:) 100.00% (1/1) SplitTree.viewBounds() 100.00% (4/4) SplitTree.init(from:) 76.00% (19/25) SplitTree.encode(to:) 100.00% (15/15) SplitTree.Node.find(id:) 100.00% (13/13) SplitTree.Node.node(view:) 88.89% (16/18) SplitTree.Node.path(to:) 100.00% (32/32) search #1 <A>(_:) in SplitTree.Node.path(to:) 100.00% (27/27) SplitTree.Node.node(at:) 89.47% (17/19) SplitTree.Node.inserting(view:at:direction:) 86.84% (33/38) SplitTree.Node.replacingNode(at:with:) 100.00% (43/43) replaceInner #1 <A>(current:pathOffset:) in SplitTree.Node.replacingNode(at:with:) 96.67% (29/30) SplitTree.Node.remove(_:) 70.27% (26/37) implicit closure #1 in SplitTree.Node.remove(_:) 100.00% (1/1) SplitTree.Node.resizing(to:) 100.00% (16/16) SplitTree.Node.leftmostLeaf() 87.50% (7/8) SplitTree.Node.rightmostLeaf() 87.50% (7/8) SplitTree.Node.equalize() 100.00% (4/4) SplitTree.Node.equalizeWithWeight() 100.00% (30/30) SplitTree.Node.weightForDirection(_:) 83.33% (10/12) SplitTree.Node.calculateViewBounds(in:) 100.00% (50/50) SplitTree.Node.viewBounds() 100.00% (26/26) SplitTree.Node.spatial(within:) 100.00% (18/18) SplitTree.Node.dimensions() 80.77% (21/26) SplitTree.Node.spatialSlots(in:) 100.00% (53/53) SplitTree.Spatial.slots(in:from:) 100.00% (47/47) closure #1 in SplitTree.Spatial.slots(in:from:) 100.00% (1/1) distance #1 <A>(from:to:) in SplitTree.Spatial.slots(in:from:) 100.00% (6/6) closure #2 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) implicit closure #1 in closure #2 in SplitTree.Spatial.slots(in:from:) 100.00% (1/1) closure #3 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) closure #4 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) implicit closure #1 in closure #4 in SplitTree.Spatial.slots(in:from:) 100.00% (1/1) closure #5 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) closure #6 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) implicit closure #1 in closure #6 in SplitTree.Spatial.slots(in:from:) 100.00% (1/1) closure #7 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) closure #8 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) implicit closure #1 in closure #8 in SplitTree.Spatial.slots(in:from:) 100.00% (1/1) closure #9 in SplitTree.Spatial.slots(in:from:) 100.00% (3/3) SplitTree.Spatial.doesBorder(side:from:) 100.00% (20/20) closure #1 in SplitTree.Spatial.doesBorder(side:from:) 100.00% (1/1) closure #2 in SplitTree.Spatial.doesBorder(side:from:) 100.00% (3/3) static SplitTree.Node.== infix(_:_:) 100.00% (13/13) SplitTree.Node.init(from:) 66.67% (12/18) SplitTree.Node.encode(to:) 100.00% (11/11) SplitTree.Node.leaves() 100.00% (9/9) SplitTree.makeIterator() 100.00% (3/3) implicit closure #1 in SplitTree.makeIterator() 100.00% (1/1) SplitTree.Node.makeIterator() 0.00% (0/3) SplitTree.startIndex.getter 100.00% (3/3) SplitTree.endIndex.getter 100.00% (3/3) implicit closure #1 in SplitTree.endIndex.getter 100.00% (1/1) SplitTree.subscript.getter 100.00% (5/5) implicit closure #1 in SplitTree.subscript.getter 100.00% (1/1) implicit closure #2 in implicit closure #1 in SplitTree.subscript.getter 100.00% (1/1) implicit closure #3 in SplitTree.subscript.getter 0.00% (0/1) implicit closure #4 in SplitTree.subscript.getter 0.00% (0/1) SplitTree.index(after:) 100.00% (4/4) implicit closure #1 in SplitTree.index(after:) 100.00% (1/1) implicit closure #2 in SplitTree.index(after:) 0.00% (0/1) SplitTree.Node.structuralIdentity.getter 100.00% (3/3) SplitTree.Node.StructuralIdentity.init(_:) 100.00% (3/3) static SplitTree.Node.StructuralIdentity.== infix(_:_:) 100.00% (3/3) SplitTree.Node.StructuralIdentity.hash(into:) 100.00% (3/3) SplitTree.Node.isStructurallyEqual(to:) 100.00% (18/18) implicit closure #1 in SplitTree.Node.isStructurallyEqual(to:) 100.00% (1/1) implicit closure #2 in SplitTree.Node.isStructurallyEqual(to:) 100.00% (1/1) SplitTree.Node.hashStructure(into:) 100.00% (14/14) SplitTree.structuralIdentity.getter 100.00% (3/3) SplitTree.StructuralIdentity.init(_:) 100.00% (4/4) static SplitTree.StructuralIdentity.== infix(_:_:) 100.00% (4/4) implicit closure #1 in static SplitTree.StructuralIdentity.== infix(_:_:) 100.00% (1/1) SplitTree.StructuralIdentity.hash(into:) 80.00% (8/10) static SplitTree.StructuralIdentity.areNodesStructurallyEqual(_:_:) 90.00% (9/10) </pre> </details> I chose this as a good place to start contributing to Ghostty because I was curious about the macOS implementation, and there was a specific request for help with testing (ghostty-org#7879). My process for writing the tests was basically reading [SplitTree.swift](./macos/Sources/Features/Splits/SplitTree.swift) to understand it, then writing tests for each high-level method and checking against code coverage to capture all the code paths: ## Running ```bash rm -rf /tmp/ghostty-test.xcresult xcodebuild -project macos/Ghostty.xcodeproj \ -scheme GhosttyTest \ -configuration Debug \ test \ -destination 'platform=macOS' \ -enableCodeCoverage YES \ -resultBundlePath /tmp/ghostty-test.xcresult \ -only-testing:GhosttyTests/SplitTreeTests \ 2>&1 | xcbeautify ``` ## Coverage ```bash xcrun xccov view --report /tmp/ghostty-test.xcresult | grep 'SplitTree\.' ``` This was originally implemented in [~38 commits](https://github.com/pouwerkerk/ghostty/pull/1/commits), but I squashed them down to 1 commit for easier review. ## AI Disclosure The tests were written by me, but I used Opus 4.6 to explain some parts of the code, and then finally to provide feedback on the tests. It suggested tests for `nodeStructuralIdentityInSet` and `nodeStructuralIdentityDistinguishesLeaves` as well as [the Parameterized test](pouwerkerk@6a0bca4), `resizingAdjustsRatio`, which seemed like a clever way to collapse 12 individual tests into 3 parameterized ones that still run 12 cases total. I didn't know this feature existed, and it seems like a great way to write tests that are more maintainable. I read this relatively new feature in the [Swift Docs](https://developer.apple.com/documentation/testing/parameterizedtesting). I find this to be a particularly useful feature of Claude/related agents, where it can suggest better ways of writing something in a more idiomatic way, and it taught me something new, which is always fun. I'm more than happy to continue work on tests for ghostty-org#7879 and always welcome to any feedback you have.
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
2 participants
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.
Summary
momentaryPushIntopushOnPushOffso it visually reflects whether the diff sidebar is opensetDiffSidebarButtonState(_:)toTerminalWindowand calls it fromtoggleGitDiffSidebarandcloseGitDiffinTerminalControllerTest plan
zig build)zig build test)closeGitDiff) → verify button resets to unselectedMade with Cursor