Fixes regression from #7523
I messed two things up around spatial navigation in the split tree
that this commit fixes:
1. The distance in the spatial tree only used a single dimension
that we were navigating. This commit now uses 2D euclidean
distance from the top-left corners of nodes. This handles the case
where the nodes are directly above or below each other better.
2. The spatial slots include split containers because they are layout
elements. But we should only navigate to leaf nodes. This was
causing the wrong navigatin to happen in some scenarios.
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!
Conversion of #7497 to a PR. This implements a feature requested in
#7331: an option to hide the default window buttons on macOS for a
cleaner aesthetic.
~~Builds on #7502 as it requires the same change to avoid the main
toolbar title showing on top of the tab bar.~~ EDIT: rebased on main now
that #7502 was merged.
I aligned the scope of the new option with `macos-titlebar-style`, since
they both customize titlebar elements. This means it has the same edge
case quirks: For example, if you change the setting, reload the config,
and then open a new tab, the appearance of the current window will
depend on which tab is in the foreground. I did it this way because
`macos-titlebar-style` provided an easy template for which derived
configs and functions to modify. Let me know if you want me to try
adjusting this so that a change in the setting also takes effect for
current windows/tabs, which I _think_ should be possible.
Screenshots:
* `macos-titlebar-style = transparent` (default)
* `macos-titlebar-style = tabs`
`TabView` assumes to be the sole owner of all `Tab`s within a Window. As
such, it could close the managed `Window` once all tabs are removed from
its widget.
However, during `AdwTabView::close-page` signal triggered by libadwaita,
the `Tab` to be closed will gain an another reference for the duration
of the signal, breaking `TabView.closeTab` (called via
`Tab.closeWithConfirmation`) assumptions that having no tabs meant they
are all destroyed.
This commit solves the issue by scheduling `Tab.closeWithConfirmation`
to be run after `AdwTabView::close-page` signal has finished processing.
Fixes#7426
First, remove the always-inlined openTerminalFromPasteboard code and
combine it with openTerminal. Now that we're doing a bit of work inside
openTerminal, there's little better to having an intermediate, inlined
function.
Second, combine some type-casting operations (saving a .map() call).
Lastly, adjust some variable names because a generic `objs` or `urls`
was a little ambiguous now that we're all in one function scope.
Bumps
[namespacelabs/nscloud-cache-action](https://github.com/namespacelabs/nscloud-cache-action)
from 1.2.7 to 1.2.8.
<details>
<summary>Commits</summary>
<ul>
<li><a
href="449c929cd5"><code>449c929</code></a>
Merge pull request <a
href="https://redirect.github.com/namespacelabs/nscloud-cache-action/issues/20">#20</a>
from namespacelabs/niklas-timeout</li>
<li><a
href="a63596ed5b"><code>a63596e</code></a>
Wrap action with a timeout.</li>
<li>See full diff in <a
href="https://github.com/namespacelabs/nscloud-cache-action/compare/v1.2.7...v1.2.8">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
The current documentation for actions are very sparse and would leave
someone (even including contributors) as to what exactly they do. On top
of that there are many stylistic and grammatical problems that are
simply no longer in line with our current standards, and certainly not
on par with our configuration options reference.
Hence, I've taken it upon myself to add, clarify, supplement, edit and
even rewrite the documentation for most of these actions, in a wider
effort of trying to offer better, clearer documentation for our users.
The current documentation for actions are very sparse and would leave
someone (even including contributors) as to what exactly they do.
On top of that there are many stylistic and grammatical problems that are
simply no longer in line with our current standards, and certainly not
on par with our configuration options reference.
Hence, I've taken it upon myself to add, clarify, supplement, edit and
even rewrite the documentation for most of these actions, in a wider
effort of trying to offer better, clearer documentation for our users.
`TabView` assumes to be the sole owner of all `Tab`s within a Window.
As such, it could close the managed `Window` once all tabs are removed
from its widget.
However, during `AdwTabView::close-page` signal triggered by libadwaita,
the `Tab` to be closed will gain an another reference for the duration
of the signal, breaking `TabView.closeTab` (called via
`Tab.closeWithConfirmation`) assumptions that having no tabs meant they
are all destroyed.
This commit solves the issue by scheduling `Tab.closeWithConfirmation`
to be run after `AdwTabView::close-page` signal has finished processing.
This is done to match against the default application id when Ghostty is
built using debug configuration, preparing the Flatpak version for D-Bus
activation support (#7433).
This is done to match against the default application id when Ghostty is
built using debug configuration, done to prepare the Flatpak version for
D-Bus activation support.
First, remove the always-inlined openTerminalFromPasteboard code and
combine it with openTerminal. Now that we're doing a bit of work inside
openTerminal, there's little better to having an intermediate, inlined
function.
Second, combine some type-casting operations (saving a .map() call).
Lastly, adjust some variable names because a generic `objs` or `urls`
was a little ambiguous now that we're all in one function scope.
This fixes a small memory leak I found where the `SplitNode.Leaf` was
not being deinitialized properly when closing a split. It would get
deinitialized the next time a split was made or the window was closed,
so the leak wasn't big. The surface view underneath the split was also
properly deinitialized because we forced it, so again, the leak was
quite small.
But conceptually this is a big problem, because when we change the
surface tree we expect the deinit chain to propagate properly through
the whole thing, _including_ to the SurfaceView.
This fixes that by removing the `id(node)` call. I don't find this to be
necessary anymore. I don't know when that happened but we've changed
quite a lot in our split system since it was introduced. I'm also not
100% sure why the `id(node)` was causing a strong reference to begin
with... which bothers me a bit.
AI note: While I manually hunted this down, I started up Claude Code and
Codex in separate tabs to also hunt for the memory leak. They both
failed to find it and offered solutions that didn't work.
This fixes a small memory leak I found where the `SplitNode.Leaf` was
not being deinitialized properly when closing a split. It would get
deinitialized the next time a split was made or the window was closed,
so the leak wasn't big. The surface view underneath the split was also
properly deinitialized because we forced it, so again, the leak was
quite small.
But conceptually this is a big problem, because when we change the
surface tree we expect the deinit chain to propagate properly through
the whole thing, _including_ to the SurfaceView.
This fixes that by removing the `id(node)` call. I don't find this to be
necessary anymore. I don't know when that happened but we've changed
quite a lot in our split system since it was introduced. I'm also not
100% sure why the `id(node)` was causing a strong reference to begin
with... which bothers me a bit.
AI note: While I manually hunted this down, I started up Claude Code and
Codex in separate tabs to also hunt for the memory leak. They both
failed to find it and offered solutions that didn't work.
Just a quick follow-up to #7462: I noticed Swift was admonishing me
about a `var` that could be a `let`, and realized that it was I who had
failed to make that change. Apologies for the noise.
