Repository navigation
linux(socket): implement pane.resize via split tree (Sprint A #3) - #228
Conversation
Adds pane.resize to the v2 socket protocol on Linux. Walks the split
tree to find the innermost ancestor split node matching the requested
direction (left/right → horizontal, up/down → vertical) and adjusts
its ratio by amount * 0.05 per step, clamped to [0.1, 0.9].
New split_tree.zig helpers:
- containsPanel: check if a subtree contains a given panel id
- findResizeSplit: find the innermost matching split ancestor for a
given pane id, orientation, and first/second-child position
The handler resolves the target pane from pane_id (or focused), maps
direction to orientation + child position, finds the split, adjusts
its ratio, and in real GTK mode updates the GtkPaned widget position.
Returns {pane_id, direction, ratio} with the new ratio.
Adds tests_v2/test_pane_resize.py covering: ratio increase/decrease,
multi-step amount, clamping at 0.1/0.9 bounds, single-pane error,
bad direction error, and no-matching-split-ancestor error.
Refs #220 (Sprint A item #3).
Greptile SummaryImplements The Confidence Score: 5/5Safe to merge — no P0/P1 findings; implementation is logically correct and the fix commit resolves a real compile error. All three findings are P2 (error message clarity, missing isinstance guards in tests, uncovered focused-pane path). The core tests_v2/test_pane_resize.py — minor robustness and coverage gaps, but nothing blocking. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[pane.resize RPC] --> B{workspace_id?}
B -- yes --> C[findWorkspaceById]
B -- no --> D[selectedWorkspace]
C --> E{pane_id?}
D --> E
E -- yes --> F[findSurfaceInWorkspace]
E -- no --> G[ws.focused_panel_id]
F --> H{root_node?}
G --> H
H -- null --> I[error: no split tree]
H -- leaf/split --> J[findResizeSplit]
J --> K{match found?}
K -- null --> L["error: no {orientation} split ancestor"]
K -- Split ptr --> M[clamp ratio to 0.1-0.9]
M --> N{isNoSurface?}
N -- false --> O{split.paned != null?}
O -- yes --> P[gtk_paned_set_position]
O -- no --> Q[skip GTK update]
N -- true --> R[skip GTK update]
P --> S["return {pane_id, direction, ratio}"]
Q --> S
R --> S
Reviews (2): Last reviewed commit: "fix(socket): use .empty + alloc for Arra..." | Re-trigger Greptile |
| surfaces_2 = c.list_surfaces() | ||
| if len(surfaces_2) < 2: | ||
| raise cmuxError(f"expected >= 2 surfaces after split, got {len(surfaces_2)}") | ||
| s0_id = surfaces_2[0][1] | ||
| s1_id = surfaces_2[1][1] |
There was a problem hiding this comment.
Split-tree ordering assumed but not verified
s0_id and s1_id are assigned from list_surfaces() index positions, implicitly assuming the list returns panes in split-tree order (first child before second child). If the surface list is ordered by insertion time or some other criterion, the "resize right from s0" assertion can silently test the wrong pane. A quick guard — comparing s1_id against the s1 surface-id returned by new_split — would make the assumption explicit and catch ordering regressions:
s1 = c.new_split("right")
time.sleep(0.1)
surfaces_2 = c.list_surfaces()
s0_id = surfaces_2[0][1]
s1_id = surfaces_2[1][1]
# Verify list ordering matches split position
if s1_id != s1:
raise cmuxError(f"expected s1_id={s1!r}, list returned {s1_id!r} as second surface")| # Resize up/down on a horizontal-only split → no matching ancestor | ||
| _expect_error( | ||
| "no vertical split", | ||
| lambda: c._call( | ||
| "pane.resize", | ||
| {"workspace_id": ws, "pane_id": s0_id, "direction": "up"}, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
No positive test for vertical splits
The test verifies that up/down on a horizontal-only tree returns an error, but never exercises the happy path (ratio increase/decrease on an actual vertical split). A brief addition after the horizontal tests — new_split("down"), then a pane.resize with direction: "down" or "up" — would confirm findResizeSplit returns the right ancestor for .vertical orientation, not just that it fails correctly when none exists.
| if (!isNoSurface()) { | ||
| if (split.paned) |paned_widget| { | ||
| const alloc_size = if (orientation == .horizontal) | ||
| c.gtk.gtk_widget_get_width(paned_widget) | ||
| else | ||
| c.gtk.gtk_widget_get_height(paned_widget); | ||
| if (alloc_size > 0) { | ||
| const pos: c_int = @intFromFloat(split.ratio * @as(f64, @floatFromInt(alloc_size))); | ||
| c.gtk.gtk_paned_set_position(@ptrCast(@alignCast(paned_widget)), pos); | ||
| } | ||
| } |
There was a problem hiding this comment.
Silent no-op when
split.paned is null in real GTK mode
If a split node exists in the model tree but buildWidget has not yet been called for it (e.g. the workspace was created programmatically and not yet laid out), split.paned is null and the GTK position update is silently skipped. The model ratio is still mutated, so subsequent calls will round-trip correctly, but the live GtkPaned widget won't move until some other GTK event causes a relayout. A log.warn on the null branch would surface this in debug builds:
if (split.paned) |paned_widget| {
// ... existing position update ...
} else {
log.warn("pane.resize: split.paned is null, GTK position not updated", .{});
}The close_left/close_right/close_others handler used the old .init(alloc) and .deinit() API that doesn't exist in this Zig version. Match the codebase pattern: .empty + .deinit(alloc) + .append(alloc, id).
Summary
Implements the v2 `pane.resize` RPC on Linux by walking the split tree
to find the innermost ancestor split node matching the requested direction
and adjusting its divider ratio.
How it works
What's added
given orientation and first/second-child position
Linux vs macOS
macOS computes delta from pixel dimensions
(`amount / axisPixels`). Linux uses a fixed `0.05` step per unit since
CMUX_NO_SURFACE mode has no real pixel allocations. In real GTK mode
the ratio is applied to the GtkPaned's actual allocation.
Test plan
Refs #220 (Sprint A item #3).