Refactor index tuples to structs#312
Conversation
anhosh
left a comment
There was a problem hiding this comment.
Thank you, I really like this changeset, it cleans up a lot of code and adds some quite useful iterators. I only have a few comments which I've posted.
|
I am removing the Into conversions from *Path to *Index since that would create confusing APIs such as let third_tab = NodePath::new(NodeIndex(2), TabIndex(3));
tree.set_active_tab(third_tab, TabIndex(4))which reads really weird since we are'nt actually setting the active tab to |
7110ef1 to
e26ff6b
Compare
|
Fixed. This would be a breaking change btw, so a separate version bump would be required before tagging. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the docking API to replace (SurfaceIndex, NodeIndex, TabIndex) tuple addressing with explicit NodePath / TabPath structs, and introduces fallible accessors (Result/Error) for safer node/tab operations.
Changes:
- Introduce
NodePathandTabPathtypes and migrate tab/node movement/removal/drag-and-drop code to use them. - Add
dock_state::Error/Resultand make “set active tab” operations fallible instead of silently ignoring invalid indices. - Update public
TabViewerhooks and examples to useNodePath.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/widgets/tab_viewer.rs | Update TabViewer callbacks (context_menu, on_add, add_popup) to accept NodePath. |
| src/widgets/dock_area/tab_removal.rs | Refactor removal queue entries to store NodePath / TabPath. |
| src/widgets/dock_area/state.rs | Import order cleanup. |
| src/widgets/dock_area/show/mod.rs | Update docking UI logic to use NodePath/TabPath for removals, detach, and DnD sources. |
| src/widgets/dock_area/show/leaf.rs | Convert leaf rendering, context menus, detach/close paths, and focus tracking to NodePath/TabPath. |
| src/widgets/dock_area/mod.rs | Update internal DockArea state (to_detach, new_focused) to store path structs. |
| src/widgets/dock_area/drag_and_drop.rs | Refactor DnD TreeComponent and destination mapping to path structs. |
| src/utils.rs | Import ordering change. |
| src/dock_state/tree/tab_index.rs | Add TabPath and related helpers/conversions. |
| src/dock_state/tree/node_index.rs | Add NodePath and helpers (MAIN_ROOT, left_node, right_node). |
| src/dock_state/tree/node/mod.rs | Add indexed tab iterators (iter_tabs_indexed, iter_tabs_mut_indexed). |
| src/dock_state/tree/node/leaf.rs | Make LeafNode::set_active_tab return Result with Error::InvalidTab. |
| src/dock_state/tree/mod.rs | Re-export new path types; update TabDestination::Node to carry NodePath; add safe leaf/leaf_mut; make Tree::set_active_tab fallible. |
| src/dock_state/surface.rs | Add indexed node iterators and return tab indices from iter_all_tabs(_mut). |
| src/dock_state/mod.rs | Add Error/Result; add safe accessors (node, node_mut, leaf, leaf_mut); refactor move/detach/remove APIs to NodePath/TabPath; make DockState::set_active_tab fallible. |
| src/dock_state/error.rs | New error type used by fallible dock/tree operations. |
| examples/text_editor.rs | Handle fallible set_active_tab via expect. |
| examples/tab_add_popup.rs | Update example tab metadata and TabViewer hook signatures to use NodePath. |
| examples/tab_add.rs | Update add-button plumbing to use NodePath. |
| examples/simple.rs | Minor formatting/import cleanup. |
| examples/save_load_dock_state.rs | Import order cleanup. |
| examples/reject_windows.rs | Minor formatting/import cleanup. |
| examples/hello.rs | Update context_menu signature to accept NodePath. |
| Cargo.toml | Add thiserror dependency for the new error type. |
|
copilot comments also resolved |
* impl Index<TabIndex> for LeafNode (#311) * fix(layout): prevent panics on extreme window shrink (#307) (#309) * fix: clamp tab index in move_tab to prevent out-of-bounds insert (#308) * fix: clamp tab index in move_tab to prevent out-of-bounds insert When reordering a tab within the same node, `remove_tab` is called before `insert_tab`. This reduces the tab count by one, so the original destination index may now be out of bounds. Clamp the insertion index to the current tab count to prevent panic. * Simplify the clamping --------- Co-authored-by: Adanos020 <adanos020@gmail.com> * Refactor index tuples to structs (#312) * Refactor index tuples to structs * add convenience functions on NodePath and TabPat * reorganize imports * refactor some fallible methods to return an Error --------- Co-authored-by: Adam Gąsior <adanos020@gmail.com> * Update egui/eframe to 0.34 (#314) * Update egui/eframe to 0.34 - Bump egui from 0.33 to 0.34 - Bump eframe from 0.33 to 0.34 - Deprecate DockArea::show(ctx) in favor of show_inside(ui) - Migrate all examples to eframe's new App::ui(&mut Ui) API - Replace deprecated ctx.style() with ctx.global_style() - Fix deprecated warnings in doc-tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: cargo fmt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update changelog * Update Cargo.toml and README.md * Fix clippy warnings --------- Co-authored-by: Jonathan Chan Kwan Yin <sofe2038@gmail.com> Co-authored-by: Matias Lopez <109480516+MatiasLopezING@users.noreply.github.com> Co-authored-by: Ivan <ixentrum@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
No description provided.