consolidate .children() APIs and iterators
#1156
Conversation
🦋 Changeset detectedLatest commit: a2aa67d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
OmarTawfik
changed the title
.children() APIs into iterators.children() APIs and iterators
OmarTawfik
force-pushed
the
fix-children
branch
2 times, most recently
from
9a24218
to
e90c3d4
Compare
| .cloned() | ||
| }) | ||
| .map(|identifier| identifier.text.clone()) | ||
| .filter(|node| node.is_terminal_with_kind(TerminalKind::Identifier)) |
There was a problem hiding this comment.
It's weird that node here is actually an Edge, which in turn consists of a label and a node. I think that's the most confusing part of this change in general: the items of both children() and CursorIterator (through descendants()) are now edges instead of nodes. Previously it was explicit with the added .with_edges() call to the cursor. It'll maybe catch users by surprise.
There was a problem hiding this comment.
We only had .nodes() and .edges() because edges came later, but I'm not sure it makes sense to add a specific iterator for it, given that it doesn't cost anything to do both, and they already have a Deref between them..
I updated the variable names to make this clearer. Please let me know if you have any other suggestions.
crates/metaslang/cst/src/cursor.rs
Outdated
| } | ||
| let current = Edge { | ||
| label: self.cursor.label(), | ||
| node: self.cursor.node().clone(), |
There was a problem hiding this comment.
The clone() here is redundant as the cursor's node is already cloned inside node().
| /// Returns an iterator over all descendants of the current node in pre-order traversal. | ||
| descendants: func() -> cursor-iterator; | ||
| /// Returns an iterator over all the remaining nodes in the current tree, moving in pre-order traversal, until the cursor is completed. | ||
| consume: func() -> cursor-iterator; |
There was a problem hiding this comment.
IIUC, the semantics of the generated function are different from the Rust's native function since this implementation will clone before calling to consume. Does it make sense to still call it consume in the WIT interface?
There was a problem hiding this comment.
Good point. To keep things consistent, I renamed it to remainingNodes() and modified the comment.
Consolidate redundant `Node` and `Cursor` utilities like `.children()` vs `edges()`, or `Cursor` vs `CursorWithEdges`, and expose them to WASM: - add `node.descendants()` and `cursor.descendants()` APIs to allow iterating over all descendants of the current node in pre-order traversal. - add `cursor.ancestors()` API to allow iterating over all ancestors of the current node, starting with the immediate parent, and moving upwards, ending with the root node. - add `cursor.consume()` API to allow iterating over all the remaining nodes. in the current tree, moving in pre-order traversal, until the cursor is completed. - fix `node.children()` and `parseOutput.errors()` return types.
OmarTawfik
force-pushed
the
fix-children
branch
from
e90c3d4
to
a2aa67d
Compare
Consolidate redundant
NodeandCursorutilities like.children()vsedges(), orCursorvsCursorWithEdges, and expose them to WASM:node.descendants()andcursor.descendants()APIs to allow iterating over all descendants of the current node in pre-order traversal.cursor.ancestors()API to allow iterating over all ancestors of the current node, starting with the immediate parent, and moving upwards, ending with the root node.cursor.remainingNodes()API to allow iterating over all the remaining nodes. in the current tree, moving in pre-order traversal, until the tree is completed.node.children()andparseOutput.errors()return types.