Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the tree widget implementation into the structured widget framework, replacing the old widgets::tree module with a new widgets::structured::tree API centered around a Document + Row model and adapter-based row creation.
Changes:
- Replace prompt-side tree navigation to operate on
structured::tree::Document(up/down/toggle/get). - Remove the legacy
promkit-widgets::treeimplementation and introduce a new structured tree implementation (Row,Adapter,Document,Config,State). - Update the tree example to build a
Documentfrom a filesystem path.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| promkit/src/preset/tree/evaluate.rs | Updates key/mouse navigation to call document.up/down/toggle() instead of the legacy tree cursor API. |
| promkit/src/preset/tree.rs | Switches preset construction/finalization to use the new structured::tree::{State, Document} types. |
| promkit-widgets/src/tree.rs | Removes the legacy top-level tree widget module. |
| promkit-widgets/src/tree/node.rs | Removes the legacy tree node model and associated tests. |
| promkit-widgets/src/tree/tree.rs | Removes the legacy Tree cursor implementation. |
| promkit-widgets/src/structured/tree.rs | Adds a structured tree State widget implementation for rendering visible rows. |
| promkit-widgets/src/structured/tree/document.rs | Adds a navigable Document wrapper around rows + cursor position. |
| promkit-widgets/src/structured/tree/treez.rs | Introduces Row, Adapter, row creation, and RowOperation implementation for tree navigation/visibility. |
| promkit-widgets/src/structured/tree/path.rs | Adds a filesystem PathAdapter to generate rows from PathBuf. |
| promkit-widgets/src/structured/tree/config.rs | Adds structured tree rendering config (with serde support). |
| promkit-widgets/src/structured/mod.rs | Adds a tree structured submodule behind the tree feature. |
| promkit-widgets/src/lib.rs | Expands structured module gating to include the tree feature and removes the prior top-level tree export. |
| examples/tree/src/tree.rs | Updates example to create a Document via Document::from_path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn extract(&self, current: usize, n: usize) -> Vec<Row> { | ||
| let mut result = Vec::new(); | ||
| let mut index = current; | ||
|
|
||
| while index < self.len() && result.len() < n { | ||
| if is_visible(self, index) { | ||
| result.push(self[index].clone()); | ||
| } | ||
| index += 1; | ||
| } | ||
|
|
||
| result | ||
| } |
There was a problem hiding this comment.
Vec<Row>::extract linearly scans through all rows and calls is_visible() per row. When most of the tree is collapsed (common at startup), this will still traverse the entire flattened tree on every render to collect n visible rows, which can make the UI noticeably slow for large directory trees. Consider storing per-row subtree bounds (e.g., subtree_end index) and skipping whole collapsed ranges, similar to how the JSON/YAML structured widgets jump over collapsed containers, or maintain a separate list of visible row indices updated on toggle.
| pub depth: usize, | ||
| /// Display label of the current node. | ||
| pub id: String, | ||
| /// Breadcrumb-like labels from the root to this node. | ||
| pub path: Vec<String>, | ||
| /// Whether this row has child nodes. | ||
| pub has_children: bool, | ||
| /// Whether the children of this row are currently collapsed. | ||
| pub collapsed: bool, | ||
| } |
There was a problem hiding this comment.
Row stores both id and a path: Vec<String> that (per collect_rows_with) includes the current node label, so the current label is duplicated for every row. Additionally, cloning current_path into every row can become a large memory cost for big trees. To reduce duplication, consider storing only one of these (e.g., keep id and store parent indices so the full path can be reconstructed on demand), or store path without the terminal segment.
| #[cfg(any(feature = "json", feature = "yaml", feature = "tree"))] | ||
| pub mod structured; |
There was a problem hiding this comment.
With the tree functionality moved under structured, enabling the tree feature no longer provides a top-level promkit_widgets::tree module. This is a breaking public API change for downstream users. If backward compatibility is desired, consider adding a pub use structured::tree as tree; (behind the tree feature) or a small shim module re-exporting the previous public types.
No description provided.