Windows with `macos-titlebar-style = hidden` create new windows when the
new tab binding is pressed. This behavior has existed for a long time.
However, these windows did not cascade, meaning they'd appear overlapped
directly on top of the previous window, which is kind of nasty.
This commit changes it so that new windows created via new tab from a
hidden titlebar window will cascade.
Fixes#7546
SwiftUI uses type and structure to identify views, which can lead
to issues with tree like structures where the shape and type is the same
but the content changes. This was causing #7546.
To fix this, we need to add explicit identity to the split tree view
so that SwiftUI can differentiate when it needs to redraw the view.
We don't want to blindly add Hashable to SplitTree because we don't want
to take into account all the fields. Instead, we add an explicit
"structural identity" to the SplitTreeView that can be used by SwiftUI.
When closing a window that contains multiple tabs, the undo operation
now properly restores all tabs as a single tabbed window rather than
just restoring the active tab.
The implementation:
- Collects undo states from all windows in the tab group before closing
- Sorts them by their original tab index to preserve order
- Clears tab group references to avoid referencing garbage collected objects
- Restores all windows and re-adds them as tabs to the first window
- Tracks and restores which tab was focused (or focuses the last tab if none were)
AI prompts that generated this commit are below.
Each separate prompt is separated by a blank line, so this session was
made up with many prompts in a back-and-forth conversation.
> We need to update the undo/redo implementation in
> @macos/Sources/Features/Terminal/TerminalController.swift `closeWindowImmediately`
> to handle the case that multiple windows in a tab group are closed all at once,
> and to restore them as a tabbed window. To do this, I think we should collect
> all the `undoStates`, sort them by `tabIndex` (null at the end), and then on j
> restore, restore them one at a time but add them back to the same tabGroup. We
> can't use the tab group in the `undoState` because it will be garbage collected
> by then. To be sure, we should just set it to nil.
I should note at this point that the feature already worked, but the
code quality and organization wasn't up to my standards. If someone
using AI were just trying to make something work, they might be done at
this point.
I do think this is the biggest gap I worry about with AI-assisted
development: bridging between the "it works" stage at a junior quality
and the "it works and is maintainable" stage at a senior quality. I
suspect this will be a balance of LLMs getting better but also senior
code reviewers remaining highly involved in the process.
> Let's extract all the work you just did into a dedicated private method
> called `registerUndoForCloseWindow`
Manual: made some tweaks to comments, moved some lines around, didn’t change
any logic.
> I think we can pull the tabIndex directly from the undoState instead of
> storing it in a tuple.
> Instead of `var undoStates`, I think we can create a `let undoStates` and
> build and filter and sort them all in a chain of functional mappings.
> Okay, looking at your logic for restoration, the `var firstController` and
> conditionals are littly messy. Can you make your own pass at cleaning those
> up and I'll review and provide more specific guidance after.
> Excellent. Perfect. The last thing we're missing is restoring the proper
> focused window of the tab group. We should store that and make sure the
> proper window is made key. If no windows were key, then we should make the
> last one key.
> Excellent. Any more cleanups or comments you'd recommend in the places you
> changed?
Notes on the last one: it gave me a bunch of suggestions, I rejected most but
did accept some.
> Can you write me a commit message summarizing the changes?
It wrote me a part of the commit message you're reading now, but I
always manually tweak the commit message and add my own flair.
This fixes a regression from the new split work last week, but it was
also probably an issue before that in a slightly different way.
With the new split work, the quick terminal was becoming unusable when
the final surface explicitly `exit`-ed, because AppKit/SwiftUI would
resize the window to a very small size and you couldn't see the new
terminal on the next toggle.
Prior to this, I think the quick terminal would've reverted to its
original size but I'm not sure (even if the user resized it manually).
This commit saves the size of the quick terminal at the point all
surfaces are exited and restores it when the quick terminal is shown the
next time with a new initial surface.
