Merge pull request #594 from mitchellh/gtk-prev-tab

apprt/gtk: ctrl+page-up/down for prev/next tab, does not consume input if no tabs

src/Surface.zig

@@ -927,15 +927,16 @@ 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});
-        if (event.action == .press or event.action == .repeat) {
-            try self.performBindingAction(binding_action);
-        }
+        log.debug("key event binding consumed={} action={}", .{ consumed, 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

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;
         };

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);
 }

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;
     };

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 } },
@@ -1537,6 +1547,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),
             },
         };
     }