Add document tree iterator #251
Merged
Conversation
This file contains 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
Kiyoshika
commented
Nov 11, 2023
| @@ -185,20 +185,28 @@ impl TreeBuilder for DocumentTaskQueue { | |||
| new_id | |||
| } | |||
|
|
|||
| fn create_text(&mut self, content: &str, parent_id: NodeId) { | |||
| fn create_text(&mut self, content: &str, parent_id: NodeId) -> NodeId { | |||
There was a problem hiding this comment.
Making create_text and create_comment return a node ID is not mandatory but I did it for convenience (in the tests) and also it makes the tree builder interface more consistent anyways..
Kiyoshika
commented
Nov 11, 2023
|
|
||
| while let Some(current_node_id) = node_stack.pop() { | ||
| for current_node_id in tree_iterator { |
There was a problem hiding this comment.
cool side effect of using the iterator, we get this nice Python-like iteration syntax
Kiyoshika
commented
Nov 11, 2023
Kiyoshika
force-pushed
the
feat/tree-iterator
branch
2 times, most recently
from
60d6aa8
to
87528b9
Compare
neuodev
approved these changes
Nov 11, 2023
jaytaph
approved these changes
Nov 11, 2023
emwalker
reviewed
Nov 11, 2023
| let mut traversed_nodes = Vec::new(); | ||
| for current_node_id in tree_iterator { | ||
| traversed_nodes.push(current_node_id); | ||
| } |
There was a problem hiding this comment.
Also:
let traversed_nodes = tree_iterator.collect::<Vec<_>>();
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds a
TreeIteratorwhich takes a&DocumentHandleand constructs an iterator which walks through the tree one node at a time in tree order.EDIT: below statement is incorrect, mutations are reflected in the iterator if it's still kept "open." I added a warning in the struct description about this.
This iterator is a snapshot of the document at the time the iterator is created and is not updated from mutations. In that case, a new iterator must be created.At the moment, the iterator returns
NodeIds (thanks for the help from @emwalker) which is good enough for now but ideally I would like this to return some type of&NodeorRc<Node>that the user agent can use directly without having to touch the document's "internals"DocumentHandle.query()has been updated to use this new iterator as well which still passes all tests