Skip to content

Fix worktrunk sidebar flicker and selection#1

Merged
nicosuave merged 1 commit into
mainfrom
sidebar-sticky
Feb 4, 2026
Merged

Fix worktrunk sidebar flicker and selection#1
nicosuave merged 1 commit into
mainfrom
sidebar-sticky

Conversation

@nicosuave
Copy link
Copy Markdown
Member

@nicosuave nicosuave commented Feb 4, 2026

Summary:

  • Batch worktrunk refresh and snapshot sidebar data to reduce flicker.
  • Prune selection on collapse, including tab-group sync.

Tests:

  • zig build test (fails: xcodebuild test CoreSimulator connection invalid, sandbox-exec operation not permitted)

@nicosuave nicosuave merged commit 20ad06a into main Feb 4, 2026
68 checks passed
nicosuave pushed a commit that referenced this pull request Feb 20, 2026
@mitchellh
Add MockView and SplitTreeTests (ghostty-org#10778)
dada3cd 
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.
nicosuave pushed a commit that referenced this pull request Feb 26, 2026
@mitchellh
terminal: change cell width when wider grapheme detected (ghostty-org…
d186613 
…#10465)

This PR updates the logic in Terminal `print` to include more cases of
changing a cell to be wide due to a grapheme cluster that needs to be
wide but starts off narrow. The existing case of this is a
text-presentation code point followed by VS16 to make it emoji
presentation. This PR handles more cases that are found in scripts such
as Devanagari where the correct grapheme width calculation sums up
multiple code points of non-zero widths. An example, as seen from
[uucode's issue #1](jacobsandlund/uucode#1) is
`क्‍ष`, which now with ghostty-org#9680
merged is one grapheme cluster instead of two, but the U+0915 (first
code point) is width one and U+0937 (final code point) is also width
one, and the whole cluster should be width 1 + 1 = 2. This is important
to address with the grapheme break change otherwise these scripts would
show with narrow cells, incorrectly.

Before:

<img width="680" height="124" alt="CleanShot 2026-01-27 at 10 31 24@2x"
src="https://github.com/user-attachments/assets/4ff5959d-9c14-4062-8280-83004af38495"
/>

After:

<img width="646" height="118" alt="CleanShot 2026-01-27 at 10 29 10@2x"
src="https://github.com/user-attachments/assets/3ad11afd-2141-46fb-b22b-9fa7b2546366"
/>

---

Note that the logic here just takes `width_zero_in_grapheme` and if it's
not zero width, makes the cell wide. This is actually wrong for
graphemes with `prepend` (usually/always? zero width) followed by a
character that should be narrow width, but that's affecting a much
smaller number of graphemes. To address that, we would need to run the
full `wcwidth` from `uucode` on the grapheme, and compare the width
output with the current cell's `Wide`. I figured it'd be better to
incrementally just handle the bulk of the cases with the
`width_zero_in_grapheme` check.

This also adds tests to make sure moving the cell is handled correctly,
which was not the case for the existing VS16 logic.

There's a lot of code here to handle transferring the graphemes when the
narrow cell should wrap to the next line to become wide. I'd like
feedback on the approach here before attempting to clean anything up, if
desired (pull it out into a separate method?).

AI was used in some of the uucode changes in
ghostty-org#9678 (Amp--primarily for
tests), but everything was carefully vetted and much of it done by hand.
This PR was made without AI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@nicosuave