#818: Adopt quadraui SplitTree for divider geometry — 0 uses today, ~800 lines hand-rolled - #845
Merged
JDonaghy merged 2 commits intoSep 5, 2026
Conversation
…etry GroupLayout::calculate_group_rects/dividers and WindowLayout::calculate_rects/ dividers each re-derived the same split math in two independent hand-rolled recursive passes over the tree — exactly the second-source-of-truth drift risk (#582/#452) quadraui's SplitTree::layout exists to close. Both now convert to a quadraui::SplitTree and read leaf rects + divider geometry off one layout() call; the vimcode<->quadraui SplitDirection mapping is centralised in to_quadraui_direction/from_quadraui_direction and reused by render.rs's divider_to_split, which used to carry its own copy of the same match. The render.rs hit-test/drag layer (DividerGeometry, divider_hit_test, DividerMetrics/GTK_DIVIDER_METRICS, route_divider_grab, apply_divider_drag) stays local: quadraui's SplitTreeLayout::hit_test_divider/hit_test_divider_cell only support a symmetric continuous tolerance or an exact single-cell quantized match, and can't express the asymmetric multi-cell bands vimcode's TUI backend needs (e.g. a horizontal group divider's grabbable band is the whole neighbouring tab-bar block, not a margin around a point). That gap is called out in a comment at the hit-test section for a follow-up quadraui issue rather than being papered over. No behavior change: all existing black-box divider tests (both backends — gtk::testing's window_split_divider_* and tui_main::shell_app's group_divider_*_via_shell_app) pass unchanged, confirming the SplitTree round-trip reproduces the old geometry exactly.
…ailure Blocking finding: no new tests targeted the SplitTree migration. Adds one black-box test per backend that a single 2-pane divider test can't cover: a *nested* `:vsplit`-of-`:vsplit` (GTK, WindowLayout) / nested vertical editor-group split (TUI, GroupLayout), asserting the rendered position of each of the two painted dividers by `split_index`, then dragging only the inner one and asserting the outer, sibling divider's painted column is unaffected. This is the concrete shape #582/#452 warned two independently hand-rolled recursive passes over the same tree could number or position inconsistently — the exact risk `SplitTree::layout`'s single pass (adopted by this issue) closes off. Built each test by first getting the "outer"/"inner" labeling backwards against the real tree shape (GTK's `:vsplit` nests on the *first* child, TUI's editor-group split nests on the *second*), which briefly produced a false failure with a good lesson: select the divider under test by `split_index` from `SplitTree::layout`'s own pre-order numbering, never by comparing painted x/column, since which child the nesting lands in isn't fixed. Honesty check per the issue's "state that new tests fail against unfixed develop" clause: with `src/core/window.rs`/`src/render.rs` swapped back to the pre-#818 hand-rolled two-pass code (commit f989712) and only these two new tests re-run, both still pass. That's the expected result of a genuinely behavior-preserving refactor (already argued algebraically in the original commit) — #582's fix had already made the two hand-rolled passes agree before this issue touched them, so there is no divergence left in `develop` for a black-box test to catch red-handed. Recorded here per the letter of the acceptance clause rather than silently skipped; the new tests still earn their keep as regression coverage for the `SplitTree`-based code going forward, exercising a multi-divider tree shape neither backend had any coverage for before. Nit: `window_id_from_widget`/`group_id_from_widget` now `panic!` with the offending id on parse failure instead of silently falling back to id `0` — a wrong-window bug from a future `quadraui::WidgetId` change would be far harder to spot than a loud failure. Not addressed here (needs the coordinator, not this branch): the non-blocking ask to confirm a quadraui issue was filed for the asymmetric/multi-cell divider-tolerance gap `render.rs`'s `divider_to_split` doc comment already describes, and to confirm #818 stays open behind it. This worker has no `gh`/GitHub access to file that issue directly — flagged in the final report for the coordinator to file and to keep #818 open (or explicitly re-scope it) rather than close it as fully done.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #818
Automated PR opened by coordinator for review of issue #818.