linux: implement split pane actions, surface close, and input fixes#241
linux: implement split pane actions, surface close, and input fixes#241Jesssullivan merged 8 commits intomainfrom
Conversation
Wire 5 split management actions from libghostty: - new_split: create terminal in focused pane via split_tree.splitPane - goto_split: navigate between panes (next/prev/directional) - resize_split: adjust split ratio via findResizeSplit - equalize_splits: reset all ratios to 0.5 - toggle_split_zoom: placeholder (logs, returns true) Add split_tree helpers: collectLeaves, adjacentLeaf, equalize, applyRatios. Rebuild workspace widget tree after mutations via AdwTabView page replacement with idle ratio application.
Greptile SummaryThis PR implements the full split pane action surface for the Linux client:
Confidence Score: 4/5Safe to merge after addressing the stale focused_panel_id bug; all other previously-flagged P1s are resolved. One new P1 found: after a pane closes via surface exit, focused_panel_id is not updated, leaving the workspace unable to perform any further split actions without a manual click. All previously-flagged P1s (use-after-free, orphaned panel, wrong divisor, buildWidget ordering) are correctly addressed. The remaining findings are P2 (OOM leak in splitPane, incomplete consumed_mods for symbols). cmux-linux/src/app.zig — onCloseSurface split path needs focused_panel_id update after closePane Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[onAction] --> B{Action type}
B -->|NEW_SPLIT| NS[handleNewSplit]
B -->|GOTO_SPLIT| GS[handleGotoSplit]
B -->|RESIZE_SPLIT| RS[handleResizeSplit]
B -->|EQUALIZE_SPLITS| ES[handleEqualizeSplits]
B -->|TOGGLE_SPLIT_ZOOM| TZ[handleToggleSplitZoom no-op]
B -->|MOVE_TAB| MT[handleMoveTab]
NS --> NS1[findNodeByPanel before create]
NS1 --> NS2[createTerminalPanel]
NS2 --> NS3[split_tree.splitPane]
NS3 -->|error| NS4[removePanel + restore focused_id]
NS3 -->|ok| NS5[rebuildWorkspaceWidget]
NS5 --> NS6[gtk_widget_grab_focus]
GS --> GS1[adjacentLeaf]
GS1 --> GS2[ws.focused_panel_id = leaf.panel_id]
GS2 --> GS3[gtk_widget_grab_focus]
RS --> RS1[findResizeSplit]
RS1 --> RS2[delta = amount / paned_size]
RS2 --> RS3[clamp ratio 0.1 to 0.9]
RS3 --> RS4[applyRatios idle callback carries ws_id]
MT --> MT1[adw_tab_view_reorder_page]
MT1 --> MT2[tm.selected_index = target_idx]
onClose[onCloseSurface] --> OC1{leafCount > 1?}
OC1 -->|yes| OC2[closePane + removePanel focused_panel_id NOT updated]
OC2 --> OC3[rebuildWorkspaceWidget]
OC1 -->|no| OC4[removePanel close workspace if empty]
RBW[rebuildWorkspaceWidget] --> R1[close old AdwTabPage]
R1 --> R2[detachLeavesFromParents]
R2 --> R3[split_tree.buildWidget]
R3 --> R4[adw_tab_view_insert]
R4 --> R5[g_idle_add applyRatiosIdle by ws_id]
Reviews (6): Last reviewed commit: "fix: grab GTK focus on new pane after sp..." | Re-trigger Greptile |
P1: fix use-after-free in applyRatiosIdle — carry workspace ID instead of raw root pointer, re-lookup in callback. P1: fix orphaned panel on splitPane OOM — find target node before creating panel, clean up on failure path. P2: toggle_split_zoom returns false (unhandled) until implemented. P2: log OOM in collectLeaves instead of silent swallow.
Address Greptile P2 findings from PR #240: - Set consumed_mods to include SHIFT when text was produced from a shifted keyval, preventing double-modifier effects in ghostty. - Translate GDK_MOD2_MASK (Num Lock) to GHOSTTY_MODS_NUM so numpad-aware programs see correct modifier state.
The GDK_MOD2_MASK constant is a C macro not always visible through Zig's @cImport. Define it locally as 0x10 (1 << 4), the standard value for Num Lock on Linux.
- Unparent leaf widgets before buildWidget so GTK4's parent assertion is satisfied (close old tab page first, then detach leaves from old GtkPaneds before building the new tree). - Use actual pane size instead of hardcoded 1000px divisor for resize_split delta-to-ratio conversion.
- onCloseSurface now updates the split tree when a pane is closed (shell exit, close-surface callback), promoting the sibling node and rebuilding the widget tree. - Add MOVE_TAB action handler for tab reordering via keybinds.
Greptile P1: after new_split, the data model set focused_panel_id to the new panel but GTK keyboard focus remained on the old pane. Add gtk_widget_grab_focus on the new widget after rebuild.
| if (ws.root_node) |root| { | ||
| if (split_tree.leafCount(root) > 1) { | ||
| _ = split_tree.closePane(ws.alloc, root, panel_id); | ||
| ws.removePanel(panel_id); | ||
| rebuildWorkspaceWidget(tm, ws); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Stale
focused_panel_id after pane close
removePanel removes the panel from both maps but never updates focused_panel_id (confirmed — only detachPanel does that). After this path returns, ws.focused_panel_id still equals the now-dead panel_id. On the next handleNewSplit / handleGotoSplit / handleResizeSplit, findNodeByPanel(root, focused_id) returns null and every action silently returns false. The user is left unable to split or navigate until a mouse click or other GTK focus event fires.
The fix is to find an adjacent leaf before closing and transfer focus to it:
// Find a sibling to transfer focus to before closing the pane
const next_leaf = split_tree.adjacentLeaf(root, panel_id, .next, ws.alloc);
_ = split_tree.closePane(ws.alloc, root, panel_id);
ws.removePanel(panel_id);
// Update data-model focus so split actions keep working
if (next_leaf) |l| {
ws.focused_panel_id = l.panel_id;
if (l.widget) |w| _ = c.gtk.gtk_widget_grab_focus(w);
} else {
ws.focused_panel_id = null;
}
rebuildWorkspaceWidget(tm, ws);
return;
Summary
Greptile findings addressed
applyRatiosIdle— carry workspace ID instead of raw pointersplitPanefails — find target node before creating panel, clean up on errorbuildWidgetcalled before leaf widgets are unparented — close old tab page and detach leaves from old GtkPaneds firstgtk_widget_get_width/heighttoggle_split_zoomreturned true before implemented — changed to return falsecollectLeavessilently drops leaves on OOM — addedlog.warnTest plan