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 is a major rework of how we represent, handle, and render splits in
the macOS app.
This new PR moves the split structure into a dedicated, generic
(non-Ghostty-specific) value-type called `SplitTree<V>`. All logic
associated with splits (new split, close split, move split, etc.) is now
handled by notifications on `BaseTerminalController`. The view hierarchy
is still SwiftUI but it has no logic associated with it anymore and
purely renders a static tree of splits.
Previously, the split hierarchy was owned by AppKit in a type called
`SplitNode` (a recursive class that contained the tree structure). All
logic around creating, zooming, etc. splits was handled by notification
listeners directly within the SwiftUI hierarchy. SwiftUI managed a
significant amount of state and we heavily used bindings, publishers,
and more. The reasoning for this is mostly historical: splits date back
to when Ghostty tried to go all-in on SwiftUI. Since then, we've taken a
more balanced approach of SwiftUI for views and AppKit for data and
business logic, and this has proven a lot more maintainable.
## Spatial Navigation
Previously, focus moving was handled by traversing the tree structure.
This led to some awkward behaviors. See:
https://github.com/ghostty-org/ghostty/issues/524#issuecomment-2668396095
In this PR, we now handle focus moving spatially. This means that move
"left" means moving to the visually left split (from the top-left
corner, a future improvement would be to do it from the cursor
position).
Concretely, given the following split structure:
```
+----------+-----+
| | b |
| | |
| a +-----+
| | |
| | |
| | |
| | |
|----------| d |
| c | |
| | |
+----------+-----+
```
Moving "right" from `c` now moves to `d`. Previously, it would go to
`b`. On Linux, it still goes to `b`.
## Value Types
One of the major architectural shifts is moving **purely to immutable
value types.** Whenever a split property changes such as a new split,
the ratio between splits, zoomed state, etc. we _create an entirely new
`SplitTree` value_ and replace it along the entire view hierarchy. This
is in some ways wasteful, but split hierarchies are relatively small
(even the largest I've seen in practical use are dozens of splits, which
is small for a computer). And using value types lets us get rid of a ton
of change notification soup around the SwiftUI hierarchy. We can rely on
reference counting to properly clean up our closed views.
> [!NOTE]
>
> As an aside, I think value types are going to make it a lot easier in
the future to implement features like "undo close." We can just keep a
trailing list of surface tree states and just restore them. This PR
doesn't do anything like that, but it's now possible.
## SwiftUI Simplicity
Our SwiftUI view hierarchy is dramatically simplified. See the
difference in `TerminalSplitTreeView` (new) vs `TerminalSplit` (old).
There's so much less logic in our new views (almost none!). All of it is
in the AppKit layer which is just way nicer.
## AI Notes
This PR was heavily written by AI. I reviewed every line of code that
was rewritten, and I did manually rewrite at every step of the way in
minor ways. But it was very much written in concert. Each commit usually
started as an AI agent writing the whole commit, then nudging to get
cleaned up in the right way.
One thing I found in this task was that until the last commit, I kept
the entire previous implementation around and compiling. The agent
having access to a previous working version of code during a refactor
made the code it produced as follow up in the new architecture
significantly better, despite the new architecture having major
fundamental differences in how it works!
Fixes#6999
It appears that at some point one of the operations causes focus to move
away for non-native fullscreen. We previously relied on the delegate
method to restore this but a better approach appears to handle this
directly in the fullscreen implementations. This fixes the linked issue.
I still think long term all the `Ghostty.moveFocus` stuff is a code
smell and we should be auditing all that code to see if we can
eliminate it. But this is a step in the right direction, and removes one
of those uses.
This replaces the use of our custom `Ghostty.KeyEquivalent` with
the SwiftUI `KeyboardShortcut` type. This is a more standard way to
represent keyboard shortcuts and lets us more tightly integrate with
SwiftUI/AppKit when necessary over our custom type.
Note that not all Ghostty triggers can be represented as
KeyboardShortcut values because macOS itself does not support
binding keys such as function keys (e.g. F1-F12) to KeyboardShortcuts.
This isn't an issue since all input also passes through a lower level
libghostty API which can handle all key events, we just can't show these
keyboard shortcuts on things like the menu bar. This was already true
before this commit.
Related to #6035
This implements the keybind/action portion of #5974 so that this can
have a binding and so that other apprts can respond to this and
implement it this way.
The native window drag region is driven ultimately by the window's
`contentLayoutRect`, so we can just override it in `TerminalWindow`
to return a rect the size of the full window, disabling the gesture
without causing any side effects by altering the responder chain.
This commit is quite large because it's fairly interconnected and can't
be split up in a logical way. The main part of this commit is that alpha
blending is now always done in the Display P3 color space, and depending
on the configured `window-colorspace` colors will be converted from sRGB
or assumed to already be Display P3 colors. In addition, a config option
`text-blending` has been added which allows the user to configure linear
blending (AKA "gamma correction"). Linear alpha blending also applies to
images and makes custom shaders receive linear colors rather than sRGB.
In addition, an experimental option has been added which corrects linear
blending's tendency to make dark text look too thin and bright text look
too thick. Essentially it's a correction curve on the alpha channel that
depends on the luminance of the glyph being drawn.
Fixes#4801
Our size calculation before improperly used a screens frame instead of
its visibleFrame. Additionally, we didn't properly account for origin
needing to move in order to fit the window on the screen.
Apparently, setting a frame height to high crashes AppKit. The width
gets clamped by AppKit but the height does not. Fun!
After updating to 1.0.1 I noticed something different in the terminal,
which turned out being the window borders - it appeared as if Ghostty
was using light-mode style borders (dark/black outline with a thin light
stroke at the top) instead of the entire light outline from before:
| 1.0.0 | 1.0.1 |
| - | - |
| <img width="308" alt="Screenshot 2025-01-01 at 2 28 12 PM"
src="https://github.com/user-attachments/assets/d8bc5bdd-c3b2-401c-a8ed-9da0b768cb3d"
/> | <img width="308" alt="Screenshot 2025-01-01 at 2 29 07 PM"
src="https://github.com/user-attachments/assets/fd710bed-1756-4f66-8402-bfbdd25218ab"
/> |
After digging a bit, I found #3834, which fixes fullscreen background
colors through alpha channels by appending a `withAlphaComponent(0.0)`
to `backgroundColor` - for reasons I may be entirely unaware of (since
I'm not a Swift developer), this seems to cause the dark-mode border
style to go away.
Some lines above that, I noticed the `.clear` callout from line 266,
which talks about matching Terminal.app's styles, and it _also_ has a
`withAlphaComponent` but set to `0.001` - if I understand correctly, and
the fix from #3834 works by setting the alpha component to a
_practically_ zero value, then I thought perhaps a really small number
like `0.001` could do the trick as well. This ended up working and
bringing back the right borders again.
Not sure again if this may make a difference anywhere else in the app or
bring any undesired behavior, but if anyone who is well-versed in Swift
would like chime in with more details or perhaps a better approach, I'd
greatly appreciate it!
This fixes https://github.com/ghostty-org/ghostty/issues/3535 .
There exists an issue in ghostty on mac where if you have hidden your
titlebar, then enter fullscreen, the titlebar will reappear after
exiting fullscreen.
The reason for this is that after exiting fullscreen macos reapplies
some styling on the new window created after exiting fullscreen. To
combat this we will reapply the styling to hide the titlebar after
exiting fullscreen.
Required config:
```
macos-titlebar-style = hidden
macos-non-native-fullscreen = true
```
Steps to reproduce:
- Open Ghostty
- Enter fullscreen (non-native)
- Exit fullscreen
On main you will see the titlebar reappearing after exiting fullscreen,
while that does not happen with this patch.
