The SDK published on Flathub updated to Zig 0.15.1 which broke the
Flathub build in CI. So let's install it ourselves so that we can
control the version.
This contains the various changes necessary to get the full unit test
suite passing Valgrind, and configures CI to run this.
I disabled relatively few (less than 10) tests under Valgrind because
they're way too slow: all `verifyIntegrity` tests, because those run
anyways in debug and check their own memory health, a font test that
fills out font map, and the sprite render test. Everything else runs
as-is.
I found a number of issues, most were in the tests themselves. A couple
in actual code. A funny one was some undefined memory on tabstop resize
if you exceed the default number of tabstops. I don't know any real
world program that ever even did that (memory issue aside), and that
whole file hasn't been touched since 2022, so that was funny.
No memory leaks in actual code, but a number of leaks in tests. All
resolved.
I think we're still missing some reports because of the Zig bug:
https://github.com/ziglang/zig/issues/19148 so I'm gong to audit our
codebase after this and look for cases of that.
When constraints increased or decreased the size significantly, the
fractional position was getting messed up by the scale. This change
separates that out so that it applies correctly.
I noticed this when messing around with constraints, adding this
constraint to every glyph and then running with `font-family=Arial` and
`adjust-cell-width = -35%` (if you want to reproduce this)
```zig
constraint = .{
.size_horizontal = .stretch,
.align_horizontal = .center,
.pad_left = 0.1,
.pad_right = 0.1,
};
```
The padding was disproportionately affecting thin glyphs that were
stretched a lot. The problem was that the padding was being multiplied
by the scale.
This also made it so the top or right of said thin glyphs often got
clipped off by the edge of the canvas.
Anyway I fixed it.
|Before|After|
|-|-|
|<img width="1824" height="1480" alt="image"
src="https://github.com/user-attachments/assets/32779f9d-a048-4a8c-b5ea-0e8a851d5119"
/>|<img width="1824" height="1480" alt="image"
src="https://github.com/user-attachments/assets/5bf449e5-699e-4bdc-ac96-2b776f9fb7fa"
/>|
The SDK published on Flathub updated to Zig 0.15.1 which broke the
Flathub build in CI. So let's install it ourselves so that we can
control the version.
When constraints increased or decreased the size significantly, the
fractional position was getting messed up by the scale. This change
separates that out so that it applies correctly.
This runs Valgrind on our unit test suite in CI. Since we're not
currently passing Valgrind, this will be incrementally updated with the
filters for our passing tests. Ultimately, we'll remove the filters and
run the full suite.
Valgrind is slow and hungry so this is our first and only job currently
on a large instance.
This runs Valgrind on our unit test suite in CI. Since we're not
currently passing Valgrind, this will be incrementally updated with the
filters for our passing tests. Ultimately, we'll remove the filters and
run the full suite.
Valgrind is slow and hungry so this is our first and only job currently
on a large instance.
This works around: https://github.com/ziglang/zig/issues/19148 This lets
our `test-valgrind` command catch some issues. We'll have to follow this
pattern in more places but I want to do it incrementally so things keep
passing.
I **do not** want to blindly follow this pattern everywhere. I want to
start by focusing in only on the structs that set `undefined` as default
fields that we're also about to test in isolation with Valgrind. It's
just too much noise otherwise and not a general style I'm sure of; it's
worth it for Valgrind though.
I'm making this PR separate from any fixes because the diff is so noisy
I don't want to lose the fixes in the noise. **This PR is therefore
functionally a no-op.**
This works around: https://github.com/ziglang/zig/issues/19148
This lets our `test-valgrind` command catch some issues. We'll have to
follow this pattern in more places but I want to do it incrementally so
things keep passing.
I **do not** want to blindly follow this pattern everywhere. I want to
start by focusing in only on the structs that set `undefined` as default
fields that we're also about to test in isolation with Valgrind. Its
just too much noise otherwise and not a general style I'm sure of; it's
worth it for Valgrind though.
This adds two explicit `zig build` steps: `run-valgrind` and
`test-valgrind` to run the Ghostty exe or tests under Valgrind,
respectively.
This simplifies the manual Valgrind calls in a few ways:
1. It automatically sets the CPU to baseline, which is a frequent and
requirement for Valgrind on newer CPUs, and generally safe.
2. It sets up the rather complicated set of flags to call Valgrind with,
importantly setting up our suppressions.
3. It enables pairing it with the typical and comfortable workflow of
specifying extra args (with `--`) or flags like `-Dtest-filter` for
tests.
This adds two explicit `zig build` steps: `run-valgrind` and
`test-valgrind` to run the Ghostty exe or tests under Valgrind,
respectively.
This simplifies the manual Valgrind calls in a few ways:
1. It automatically sets the CPU to baseline, which is a frequent and
requirement for Valgrind on newer CPUs, and generally safe.
2. It sets up the rather complicated set of flags to call Valgrind with,
importantly setting up our suppressions.
3. It enables pairing it with the typical and comfortable workflow of
specifying extra args (with `--`) or flags like `-Dtest-filter` for
tests.
I think, at this stage of AI, it is a common courtesy to disclose this.
In a perfect world, AI assistance would produce equal or higher quality
work than any human. That isn't the world we live in today, and in many
cases it's generating slop. I say this despite being a fan of and using
them successfully myself (with heavy supervision)! I think the major
issue is **inexperienced human drivers of AI** that aren't able to
**adequately review their generated code.** As a result, they're pull
requesting code that I'm sure they would be ashamed of if they knew how
bad it was.
The disclosure is to help maintainers assess how much attention to give
a PR. While we aren't obligated to in any way, I try to assist
inexperienced contributors and coach them to the finish line, because
getting a PR accepted is an achievement to be proud of. But if it's just
an AI on the other side, I don't need to put in this effort, and it's
rude to trick me into doing so.
**I'm a fan of AI assistance and use AI tooling myself.** But, we need
to be responsible about what we're using it for and respectful to the
humans on the other side that may have to review or maintain this code.
(In the spirit of this PR... none of this PR was AI generated. lol.)
Fixes#8187
This properly handles the scenario with our `read_text` C API when the
selection start is outside the viewport. Previously we'd report null (no
text) which would cascade into things like the right click menu not
showing a copy option.
Fixes#8187
This properly handles the scenario with our `read_text` C API when the
selection start is outside the viewport. Previously we'd report null (no
text) which would cascade into things like the right click menu not
showing a copy option.
This MR addresses #4404 following the approach suggested
[here](https://github.com/ghostty-org/ghostty/issues/4404#issuecomment-2708410143).
Implementing the behavior known e.g. from Putty or Windows Terminal.
The following configuration values for `right-click-action` are
provided:
* `context-menu` - Show the context menu.
* `paste` - Paste the contents of the clipboard.
* `copy` - Copy the selected text to the clipboard.
* `copy-and-paste` - Copy the selected text to the clipboard, paste if
nothing is selected.
* `ignore` - Do nothing, ignore the right-click.
I followed #5935 for getting an idea on where to start. I hope this to
be a temporary solution until "bindable mouse bindings" are introduced.
This is my first time writing Zig code, so I am happy to incorporate any
feedback.
Thank you all very much for your work!
The `gdk.Surface` is only ever available *after* the window had been
first presented and mapped. Trying to get the surface during `init` like
what we had previously done will **always** return null.
Previously, when encountering an OOM when copying graphemes, hyperlinks,
or styles to a new page during reflow, the attempted resolution was to
copy the current row in to a new page and continue on- which works in
99% of cases, but isn't sound, since it's possible for a single row to
exceed the capacity on any of these.
This led to rare but real crashes like #8009.
I've added tests that produce all of the failure conditions, and
resolved them by changing the strategy from making a new page to
increasing the capacity of the current one.
There should probably be some level of abstraction added around this,
since multiple places in the code now do this sort of thing- attempt to
add some managed memory to a page, adjusting their capacity upwards as
necessary. But for now, I kept it all inline here.
The `gdk.Surface` is only ever available *after* the window had been
first presented and mapped. Trying to get the surface during `init`
like what we had previously done will **always** return null.
When window-decoration=none, setting the window decoration to null would
just mean it would default to none again, creating a cycle of torment
none can break out of... that sounds a bit too dramatic doesn't it
Fixes#8274
In our webgen we treat 4 consecutive spaces as a code block, which is
often triggered by mistake when a paragraph is encased within a list.
We should probably fix this more thoroughly at some point since I don't
think actual Markdown parsers have the same behavior, but for now we
just fall back to using 3-space indents.
As an example of this occurring on the tip website:
<img width="1177" height="1012" alt="paragraphs being erroneously turned
into code blocks"
src="https://github.com/user-attachments/assets/878a8c83-3e37-41b7-90d9-fbd5b692bf16"
/>
When window-decoration=none, setting the window decoration to null would
just mean it would default to none again, creating a cycle of torment
none can break out of... that sounds a bit too dramatic doesn't it
Fixes#8274
In our webgen we treat 4 consecutive spaces as a code block, which is
often triggered by mistake when a paragraph is encased within a list.
We should probably fix this more thoroughly at some point since I don't
think actual Markdown parsers have the same behavior, but for now we
just fall back to using 3-space indents.
This fixes the incorrect comment and uses $HOME (rather than ~) to be a
little bit more explicit.
Also, our script is named ghostty-integration, not ghostty.zsh, so
update that part of the comment, too.
