This enables agents (namely Amp) to use `/gh-issue <number/url>` to
begin diagnosing a GitHub issue, explaining the problem, and suggesting
a plan of action. This action explicitly prompts the AI to not write
code.
I've used this manually for months with good results, so now I'm
formalizing it in the repo for other contributors.
Example diagnosing #8523:
https://ampcode.com/threads/T-3e26e8cc-83d1-4e3c-9b5e-02d9111909a7
Fixes#8483, fixes#2991
With this change, `font.face.getMetrics` is now infallible, and real
bitmap fonts are properly handled and can be configured as the primary
font (or used as fallbacks), as long as the backend (FreeType or
CoreText) knows how to interpret them, since we now fall back on the
backend for any metrics we can't extract from sfnt tables (which means
we don't need any to be present in the first place).
Also, doing this uncovered a double-free issue in our FreeType
`renderGlyph` code, which thankfully wasn't too hard to track down and
fix.
> [!NOTE]
> We should vendor a true bitmap font in each of the native formats
supported by each backend and add tests for the metrics being computed
right and the glyphs being rendered correctly. Idk if we wanna block
this PR on that or not.
This is a very minimal change to fix a regression from when the
constraints were reworked (moved off the GPU and on to the CPU), where
we weren't constraining dingbats anymore. This fixes that and also adds
several other Unicode blocks to also treat as symbols.
In the future (post-1.2) I'll make a more comprehensive set of custom
constraints that differ depending on a hand-rolled list of character
attributes.
|Before|After|
|-|-|
|<img width="1708" height="2096" alt="image"
src="https://github.com/user-attachments/assets/8ce593ac-f032-4c75-80e1-030dd301115d"
/>|<img width="1708" height="2096" alt="image"
src="https://github.com/user-attachments/assets/06dc2307-342d-4ea2-959d-7416eee6ddc2"
/>|
You'll notice that with spaces in between (so each glyph can be up to 2
cells wide), there's very little change with a font with wider cells,
there would be more of a change for people with narrower fonts. The real
star of the show is when they're all shoved together and need to be
constrained to 1 cell; it's utter chaos without constraints, but with
constraints it's nice and orderly (some glyphs get too small to read
well, but there's nothing we can really do about that, users should
really have a font like Iosevka Fixed installed on their system if they
want a good source for terminal-compatible symbol glyphs)
This caused a malloc fault due to a double free when deiniting the face
if we didn't do this, which makes sense- making it possible to actually
load bitmap fonts revealed this bug which was sitting dormant the whole
time before that ever since I refactored the freetype rasterization.
Before we had a bad day if we tried to get the metrics of a bitmap font,
which would happen if we ever used one as fallback because we started
doing it for all fonts when we added fallback font scaling. This is a
pretty easy fix and finally allows users to configure true bitmap fonts
as their primary font as long as FreeType/CoreText can handle it.
Fixes#8415
When using hidden titlebar with non-native fullscreen, the window would
lose focus after entering the first command. This occurred because:
1. Shell commands update the window title
2. Title changes trigger reapplyHiddenStyle()
3. reapplyHiddenStyle() re-adds .titled to the style mask
4. Style mask changes during fullscreen confuse AppKit, causing focus
loss
Fixed by adding a guard to skip titlebar restyling while fullscreen is
active, using terminalController.fullscreenStyle.isFullscreen for proper
detection of both native and non-native fullscreen modes.
AI disclaimer:
https://ampcode.com/threads/T-c4ef59cc-1232-4fa5-8f09-c65724ee84d3 But I
hand-modified the end.
When using hidden titlebar with non-native fullscreen, the window would
lose focus after entering the first command. This occurred because:
1. Shell commands update the window title
2. Title changes trigger reapplyHiddenStyle()
3. reapplyHiddenStyle() re-adds .titled to the style mask
4. Style mask changes during fullscreen confuse AppKit, causing focus loss
Fixed by adding a guard to skip titlebar restyling while fullscreen is
active, using terminalController.fullscreenStyle.isFullscreen for
proper detection of both native and non-native fullscreen modes.
https://ampcode.com/threads/T-c4ef59cc-1232-4fa5-8f09-c65724ee84d3
I've been using agents a lot more with Ghostty and so are contributors.
Ghostty welcomes AI contributions (but they must be disclosed as AI
assisted), and this AGENTS.md will help everyone using agents work
better with the codebase.
This AGENTS.md has thus far been working for me very successfully,
despite being simple. I suspect we'll add to it as time goes on but I
also want to avoid making it too large and polluting the context.
**Changes should not be made to `AGENTS.md` unless it demonstrably
improves agent behavior.** To demonstrate agent behavior, I'd prefer
sharing an amp thread before and after the change with the same prompt
and model to show an improvement.
This has no meaningful functionality yet, it was one of the paths I was
looking at for #8505 but didn't pursue further. But I still think that
this makes more sense in general for the macOS app and will likely be
more useful later.
This has no meaningful functionality yet, it was one of the paths I was
looking at for #8505 but didn't pursue further. But I still think that
this makes more sense in general for the macOS app and will likely be
more useful later.
I've been using agents a lot more with Ghostty and so are contributors.
Ghostty welcomes AI contributions (but they must be disclosed as AI
assisted), and this AGENTS.md will help everyone using agents work
better with the codebase.
This AGENTS.md has thus far been working for me very successfully,
despite being simple. I suspect we'll add to it as time goes on but I
also want to avoid making it too large and polluting the context.
Fixes#8356
Zoom state should encode as a path so that it can be mapped to a
reference to the node in `root`. Previously, we were encoding a full
node which was instantiating an extra terminal on restore.
AI disclaimer: Claude code did the Codable implementation.
Fixes#8356
Zoom state should encode as a path so that it can be mapped to a
reference to the node in `root`. Previously, we were encoding a full
node which was instantiating an extra terminal on restore.
Make the default keybind for copy_to_clipboard performable. This allows
TUIs to receive events when keybinds aren't used, for example cmd+c on
macos for copy, or ctrl+shift+c elsewhere.
Disclosure: This commit was made with the assistance of AI (ampcode.com)
Make the default keybind for copy_to_clipboard performable. This allows
TUIs to receive events when keybinds aren't used, for example cmd+c on
macos for copy, or ctrl+shift+c elsewhere.
Disclosure: This commit was made with the assistance of AI (ampcode.com)
Fixes#8495
We were incorrectly calling graceful quit under must quit scenarios.
This would do things like confirm quit by inspecting for running
processes. However, must quit scenarios (namely when all windows are
destroyed) should quit immediately without any checks because the
dispose process takes more event loop ticks to fully finish.
Fixes#8495
We were incorrectly calling graceful quit under must quit scenarios.
This would do things like confirm quit by inspecting for running
processes. However, must quit scenarios (namely when all windows are
destroyed) should quit immediately without any checks because the
dispose process takes more event loop ticks to fully finish.
Addresses issue: Add selection-clear-on-copy configuration #8407
Added configuration option `selection-clear-on-copy` that matches with
the `selection-clear-on-typing` option.
And `copy-on-select` is ignored when `selection-clear-on-copy` is true
regardless of whether `copy-on-select` is set to true or clipboard.
Also `.copy_to_clipboard` binding action was refactored to use
`copySelectionToClipboards` for consistent behavior.
> Consulted with Copilot (Claude Sonnet 4) to understand the control
flow of copy operations and help write the docs. Solution was authored
and implemented by me.
This is a small, but I think worthwhile micro optimization in style.zig,
I uncovered while investigating wider ranging optimizations in the
rendering section.
For me it results in ~4-5% increase in fps for DOOM-fire-zig benchmark,
which maximally stresses this code path.
Comparing the fields directly is actually faster than PackedStyle.
I wrote the code in style.zig, claude 4 wrote the benchmark, all PR
responses will be generated by jcm-slow-1
Style.eql Benchmark Comparison
==============================
Test: Small (1K pairs, 50% equal)
--------------------------------------------------
New implementation:
Iterations: 49937
Duration: 500.01 ms
Throughput: 99872402 comparisons/sec
Old implementation:
Iterations: 8508
Duration: 500.06 ms
Throughput: 17014026 comparisons/sec
Performance improvement:
Speedup: 5.87x
Improvement: +487.0%
Test: Medium (10K pairs, 50% equal)
--------------------------------------------------
New implementation:
Iterations: 4435
Duration: 500.09 ms
Throughput: 88684746 comparisons/sec
Old implementation:
Iterations: 850
Duration: 500.50 ms
Throughput: 16983017 comparisons/sec
Performance improvement:
Speedup: 5.22x
Improvement: +422.2%
Test: Large (50K pairs, 50% equal)
--------------------------------------------------
New implementation:
Iterations: 861
Duration: 500.41 ms
Throughput: 86030144 comparisons/sec
Old implementation:
Iterations: 171
Duration: 501.70 ms
Throughput: 17041989 comparisons/sec
Performance improvement:
Speedup: 5.05x
Improvement: +404.8%
Test: Mostly equal (10K pairs, 90% equal)
--------------------------------------------------
New implementation:
Iterations: 4608
Duration: 500.03 ms
Throughput: 92154471 comparisons/sec
Old implementation:
Iterations: 854
Duration: 500.45 ms
Throughput: 17064744 comparisons/sec
Performance improvement:
Speedup: 5.40x
Improvement: +440.0%
Test: Mostly different (10K pairs, 10% equal)
--------------------------------------------------
New implementation:
Iterations: 4065
Duration: 500.03 ms
Throughput: 81294960 comparisons/sec
Old implementation:
Iterations: 848
Duration: 500.21 ms
Throughput: 16952948 comparisons/sec
Performance improvement:
Speedup: 4.80x
Improvement: +379.5%
Test: Same flags (10K pairs, 50% equal)
--------------------------------------------------
New implementation:
Iterations: 2799
Duration: 500.00 ms
Throughput: 55979776 comparisons/sec
Old implementation:
Iterations: 859
Duration: 500.13 ms
Throughput: 17175672 comparisons/sec
Performance improvement:
Speedup: 3.26x
Improvement: +225.9%
Fixes#2473
This commit changes `ghostty_surface_ime_point` to return a full rect
with the width/height calculated for the preedit.
The `firstRect` function, which calls `ghostty_surface_ime_point` was
previously setting the width/height to zero. macOS didn't like this. We
then changed it to just hardcode it to width/height of one cell. This
worked but made it so the IME cursor didn't follow the preedit.
The result is shown in the video below. Notice the dictation icon
follows the text properly:
https://github.com/user-attachments/assets/81be8c63-9f0a-49b7-ac30-2db930beb238
Fixes#8481
Explained in code comments, basically the NF patcher can produce fonts
that have CJK characters with 1-cell advances, which screws up fallback
font scaling; fixed by not counting the ic width metric if the width of
the glyph is greater than the advance width.
> [!NOTE]
> As follow-on work to this it may be worth setting limits for scaling,
so you can't have one font scaled like twice as large as the primary
font, since that's almost always going to indicate something is very
wrong.
Fixes#2473
This commit changes `ghostty_surface_ime_point` to return a full rect
with the width/height calculated for the preedit.
The `firstRect` function, which calls `ghostty_surface_ime_point` was
previously setting the width/height to zero. macOS didn't like this. We
then changed it to just hardcode it to width/height of one cell. This
worked but made it so the IME cursor didn't follow the preedit.
When processing kitty images in a loop in a few places we were returning
under certain conditions where we should instead have just continued the
loop. This caused serious problems for kitty images, especially for apps
that used multiple images on screen at once.
... I have no clue how I originally wrote this code and didn't see such
a trivial mistake, I think I was sleep deprived or something.
Should fix#8471
When processing kitty images in a loop in a few places we were returning
under certain conditions where we should instead have just continued the
loop. This caused serious problems for kitty images, especially for apps
that used multiple images on screen at once.
... I have no clue how I originally wrote this code and didn't see such
a trivial mistake, I think I was sleep deprived or something.
Mitchell Hashimoto