Repository navigation
GTK: Don't apply unfocused options when searching - #11224
Conversation
| const priv = self.private(); | ||
| var val = gobject.ext.Value.new(bool); | ||
| defer val.unset(); | ||
| gobject.Object.getProperty( |
There was a problem hiding this comment.
We can just expose a getter for this instead of using getProperty, we can do this all via Zig without dealing with the GObject system (see other getters, even when they do both the Zig-only one is exposed). For things like primitives it's usually much easier and more ergonomic. :)
There was a problem hiding this comment.
there is already a getter I tried marking it public but it crashed on startup when I tried to use it
https://github.com/ghostty-org/ghostty/blob/main/src/apprt/gtk/class/search_overlay.zig#L284-L286
Segmentation fault at address 0xfffffffffffffe90
/home/user/workspace/ghostty/src/apprt/gtk/class/search_overlay.zig:285:30: 0x2f88459 in getSearchActive (ghostty)
return self.private().active;
^
/home/user/workspace/ghostty/src/apprt/gtk/class/surface.zig:830:64: 0x2f883b8 in closureShouldUnfocusedSplitBeShown (ghostty)
return @intFromBool(priv.search_overlay.getSearchActive() and focused == 0 and is_split != 0);
^
???:?:?: 0x7ffff54bc051 in ??? (libffi.so.8)
Unwind information for `libffi.so.8:0x7ffff54bc051` was not available, trace may be incomplete
Aborted (core dumped) ./zig-out/bin/ghostty --gtk-single-instance=false
There was a problem hiding this comment.
The crash I believe is because GTK doesn't initialize search_overlay's private Zig fields (because the creation/allocation happens in C). The correct way to do this is by passing search_overlay.active to the closure in the Blueprint. This will more accurately track changes to the properties as well.
----------------------- src/apprt/gtk/ui/1.2/surface.blp -----------------------
index 55c54531c..794ea1801 100644
@@ -203,7 +203,7 @@ Overlay terminal_page {
// Apply unfocused-split-fill and unfocused-split-opacity to current surface
// this is only applied when a tab has more than one surface
Revealer {
- reveal-child: bind $should_unfocused_split_be_shown(template.focused, template.is-split) as <bool>;
+ reveal-child: bind $should_unfocused_split_be_shown(search_overlay.active, template.focused, template.is-split) as <bool>;
transition-duration: 0;
// This is all necessary so that the Revealer itself doesn't override
// any input events from the other overlays. Namely, if you don't haveThere was a problem hiding this comment.
I assumed that you wouldn't be able to reference aother widgets members like that so I didn't try it. It seems to result in the same crash though with this diff My zig cache must have been bad it does seem to work now after clearing everything
diff --git a/src/apprt/gtk/class/surface.zig b/src/apprt/gtk/class/surface.zig
index 8d9e1bcf0..8ce9ac1d1 100644
--- a/src/apprt/gtk/class/surface.zig
+++ b/src/apprt/gtk/class/surface.zig
@@ -823,10 +823,11 @@ pub const Surface = extern struct {
/// should be applied to the surface
fn closureShouldUnfocusedSplitBeShown(
_: *Self,
+ search_active: c_int,
focused: c_int,
is_split: c_int,
) callconv(.c) c_int {
- return @intFromBool(focused == 0 and is_split != 0);
+ return @intFromBool(search_active == 0 and focused == 0 and is_split != 0);
}
pub fn toggleFullscreen(self: *Self) void {
diff --git a/src/apprt/gtk/ui/1.2/surface.blp b/src/apprt/gtk/ui/1.2/surface.blp
index 55c54531c..794ea1801 100644
--- a/src/apprt/gtk/ui/1.2/surface.blp
+++ b/src/apprt/gtk/ui/1.2/surface.blp
@@ -203,7 +203,7 @@ Overlay terminal_page {
// Apply unfocused-split-fill and unfocused-split-opacity to current surface
// this is only applied when a tab has more than one surface
Revealer {
- reveal-child: bind $should_unfocused_split_be_shown(template.focused, template.is-split) as <bool>;
+ reveal-child: bind $should_unfocused_split_be_shown(search_overlay.active, template.focused, template.is-split) as <bool>;
transition-duration: 0;
// This is all necessary so that the Revealer itself doesn't override
// any input events from the other overlays. Namely, if you don't have
mitchellh
left a comment
There was a problem hiding this comment.
Muchhhhh better thank you
If you have multiple splits and start searching naturally the focus transfers over to the search widget which would apply the unfocused options. This could make it difficult to view your matches from searching without re-focusing the surface.
This was discovered when I tested #11218 (which is a different issue)