[macos] apply swiftlint auto-fixes#8
Merged
nicosuave merged 2 commits intosidequery:mainfrom Feb 26, 2026
Merged
Conversation
Member
|
Thanks for this |
Remove redundant `= nil` on optionals (implicit_optional_initialization), prefer `private` over `fileprivate` where possible, and remove unnecessary parentheses in control statements. Co-authored-by: Cursor <cursoragent@cursor.com>
- Remove redundant string enum values in WorktreeSortOrder and WorktrunkSidebarListMode - Replace 4-member tuple with HunkRange struct in DiffParser to fix large_tuple violation Co-authored-by: Cursor <cursoragent@cursor.com>
bvolpato-dd
force-pushed
the
bv/lints
branch
from
f36fc75 to
52d6bb6
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.
Member
|
This is out in the most recent build |
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
Apply mechanical SwiftLint auto-fixes across all Swift sources in
macos/. These are safe, style-only changes with no behavioral impact.Previous PRs were surfacing lots of SwiftLint warnings — this cleans them all up so future diffs stay noise-free.
Changes
implicit_optional_initialization(40 instances, 10 files): Remove redundant= nilfrom optional property declarations (e.g.var foo: String? = nil→var foo: String?).private_over_fileprivate(1 instance): Replacefileprivatewithprivatewhere the symbol is only used within the declaring type.control_statement(1 instance): Remove unnecessary parentheses in aswitchstatement.Testing
zig buildsucceeds.zig build test(Zig + Xcode tests) all pass.swiftlint lint --fix macos.Did this cause any problems?
Rollback/revert this commit and redeploy the service.