From 659fa2cc668dd61d49abb4f1bcea56d6c2c3dc94 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 Sep 2023 22:04:17 -0700 Subject: [PATCH 1/4] config: keybinding clone must clone all members --- src/config/Config.zig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/config/Config.zig b/src/config/Config.zig index db23d0ee3..95b0681f5 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1537,6 +1537,8 @@ pub const Keybinds = struct { return .{ .set = .{ .bindings = try self.set.bindings.clone(alloc), + .reverse = try self.set.reverse.clone(alloc), + .unconsumed = try self.set.unconsumed.clone(alloc), }, }; } From a85c508892780dc41376800a5c04175002af41e8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 Sep 2023 22:04:30 -0700 Subject: [PATCH 2/4] apprt/gtk: add ctrl+page-up/down as prev/next tab --- src/Surface.zig | 2 +- src/config/Config.zig | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Surface.zig b/src/Surface.zig index e3faa8dc1..b13f3749a 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -927,7 +927,7 @@ pub fn keyCallback( // We only execute the binding on press/repeat but we still consume // the key on release so that we don't send any release events. - log.debug("key event consumed by binding action={}", .{binding_action}); + log.debug("key event binding consumed={} action={}", .{ consumed, binding_action }); if (event.action == .press or event.action == .repeat) { try self.performBindingAction(binding_action); } diff --git a/src/config/Config.zig b/src/config/Config.zig index 95b0681f5..40ea9fd08 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -602,6 +602,17 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { .{ .goto_split = .right }, ); + try result.keybind.set.putUnconsumed( + alloc, + .{ .key = .page_up, .mods = .{ .ctrl = true } }, + .{ .previous_tab = {} }, + ); + try result.keybind.set.putUnconsumed( + alloc, + .{ .key = .page_down, .mods = .{ .ctrl = true } }, + .{ .next_tab = {} }, + ); + // Viewport scrolling try result.keybind.set.put( alloc, From 56b0cb51d5957bf6d95a41f3fb9c1a188e16eeb4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 Sep 2023 22:12:38 -0700 Subject: [PATCH 3/4] apprt/gtk: previous_tab/next_tab action do not consume if there are no tabs --- src/Surface.zig | 39 ++++++++++++++++++++++++++++++++------- src/apprt/gtk/Surface.zig | 4 ++++ src/apprt/gtk/Window.zig | 11 ++++++++--- src/config/Config.zig | 21 ++++++++++----------- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index b13f3749a..5711edf08 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -928,14 +928,15 @@ pub fn keyCallback( // We only execute the binding on press/repeat but we still consume // the key on release so that we don't send any release events. log.debug("key event binding consumed={} action={}", .{ consumed, binding_action }); - if (event.action == .press or event.action == .repeat) { - try self.performBindingAction(binding_action); - } + const performed = if (event.action == .press or event.action == .repeat) + try self.performBindingAction(binding_action) + else + false; // If we consume this event, then we are done. If we don't consume // it, we processed the action but we still want to process our // encodings, too. - if (consumed) return true; + if (consumed and performed) return true; } // If this input event has text, then we hide the mouse if configured. @@ -1902,7 +1903,15 @@ fn showMouse(self: *Surface) void { /// Perform a binding action. A binding is a keybinding. This function /// must be called from the GUI thread. -pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !void { +/// +/// This function returns true if the binding action was performed. This +/// may return false if the binding action is not supported or if the +/// binding action would do nothing (i.e. previous tab with no tabs). +/// +/// NOTE: At the time of writing this comment, only previous/next tab +/// will ever return false. We can expand this in the future if it becomes +/// useful. We did previous/next tab so we could implement #498. +pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool { switch (action) { .unbind => unreachable, .ignore => {}, @@ -1971,13 +1980,13 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !void self.config.clipboard_trim_trailing_spaces, ) catch |err| { log.err("error reading selection string err={}", .{err}); - return; + return true; }; defer self.alloc.free(buf); self.rt_surface.setClipboardString(buf, .standard) catch |err| { log.err("error setting clipboard string err={}", .{err}); - return; + return true; }; } }, @@ -2116,12 +2125,26 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !void }, .previous_tab => { + if (@hasDecl(apprt.Surface, "hasTabs")) { + if (!self.rt_surface.hasTabs()) { + log.debug("surface has no tabs, ignoring previous_tab binding", .{}); + return false; + } + } + if (@hasDecl(apprt.Surface, "gotoPreviousTab")) { self.rt_surface.gotoPreviousTab(); } else log.warn("runtime doesn't implement gotoPreviousTab", .{}); }, .next_tab => { + if (@hasDecl(apprt.Surface, "hasTabs")) { + if (!self.rt_surface.hasTabs()) { + log.debug("surface has no tabs, ignoring next_tab binding", .{}); + return false; + } + } + if (@hasDecl(apprt.Surface, "gotoNextTab")) { self.rt_surface.gotoNextTab(); } else log.warn("runtime doesn't implement gotoNextTab", .{}); @@ -2163,6 +2186,8 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !void .quit => try self.app.setQuit(), } + + return true; } /// Call this to complete a clipboard request sent to apprt. This should diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 50a88401e..1e6bc907e 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -288,6 +288,10 @@ pub fn newTab(self: *Surface) !void { try self.window.newTab(&self.core_surface); } +pub fn hasTabs(self: *const Surface) bool { + return self.window.hasTabs(); +} + pub fn gotoPreviousTab(self: *Surface) void { self.window.gotoPreviousTab(self); } diff --git a/src/apprt/gtk/Window.zig b/src/apprt/gtk/Window.zig index fae1df3be..c956ffd7f 100644 --- a/src/apprt/gtk/Window.zig +++ b/src/apprt/gtk/Window.zig @@ -290,6 +290,11 @@ pub fn closeSurface(self: *Window, surface: *Surface) void { self.closeTab(page); } +/// Returns true if this window has any tabs. +pub fn hasTabs(self: *const Window) bool { + return c.gtk_notebook_get_n_pages(self.notebook) > 1; +} + /// Go to the previous tab for a surface. pub fn gotoPreviousTab(self: *Window, surface: *Surface) void { const page = c.gtk_notebook_get_page(self.notebook, @ptrCast(surface.gl_area)) orelse return; @@ -526,7 +531,7 @@ fn gtkActionClose( ) callconv(.C) void { const self: *Window = @ptrCast(@alignCast(ud orelse return)); const surface = self.actionSurface() orelse return; - surface.performBindingAction(.{ .close_surface = {} }) catch |err| { + _ = surface.performBindingAction(.{ .close_surface = {} }) catch |err| { log.warn("error performing binding action error={}", .{err}); return; }; @@ -539,7 +544,7 @@ fn gtkActionNewWindow( ) callconv(.C) void { const self: *Window = @ptrCast(@alignCast(ud orelse return)); const surface = self.actionSurface() orelse return; - surface.performBindingAction(.{ .new_window = {} }) catch |err| { + _ = surface.performBindingAction(.{ .new_window = {} }) catch |err| { log.warn("error performing binding action error={}", .{err}); return; }; @@ -552,7 +557,7 @@ fn gtkActionNewTab( ) callconv(.C) void { const self: *Window = @ptrCast(@alignCast(ud orelse return)); const surface = self.actionSurface() orelse return; - surface.performBindingAction(.{ .new_tab = {} }) catch |err| { + _ = surface.performBindingAction(.{ .new_tab = {} }) catch |err| { log.warn("error performing binding action error={}", .{err}); return; }; diff --git a/src/config/Config.zig b/src/config/Config.zig index 40ea9fd08..6d42516b6 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -561,6 +561,16 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { .{ .key = .right, .mods = .{ .ctrl = true, .shift = true } }, .{ .next_tab = {} }, ); + try result.keybind.set.put( + alloc, + .{ .key = .page_up, .mods = .{ .ctrl = true } }, + .{ .previous_tab = {} }, + ); + try result.keybind.set.put( + alloc, + .{ .key = .page_down, .mods = .{ .ctrl = true } }, + .{ .next_tab = {} }, + ); try result.keybind.set.put( alloc, .{ .key = .o, .mods = .{ .ctrl = true, .shift = true } }, @@ -602,17 +612,6 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { .{ .goto_split = .right }, ); - try result.keybind.set.putUnconsumed( - alloc, - .{ .key = .page_up, .mods = .{ .ctrl = true } }, - .{ .previous_tab = {} }, - ); - try result.keybind.set.putUnconsumed( - alloc, - .{ .key = .page_down, .mods = .{ .ctrl = true } }, - .{ .next_tab = {} }, - ); - // Viewport scrolling try result.keybind.set.put( alloc, From 8f82f8cad623be5722a18a41e11eed8ea11ff153 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 Sep 2023 22:13:34 -0700 Subject: [PATCH 4/4] apprt/embedded: adapt to new binding API --- src/apprt/embedded.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index fed00b088..36240b7f0 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -987,7 +987,7 @@ pub const CAPI = struct { return false; }; - ptr.core_surface.performBindingAction(action) catch |err| { + _ = ptr.core_surface.performBindingAction(action) catch |err| { log.err("error performing binding action action={} err={}", .{ action, err }); return false; };