Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upImplement Node::rootNode #10757
Implement Node::rootNode #10757
Conversation
highfive
commented
Apr 20, 2016
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @SimonSapin (or someone else) soon. |
highfive
commented
Apr 20, 2016
|
Heads up! This PR modifies the following files:
|
| // https://dom.spec.whatwg.org/#dom-node-rootnode | ||
| fn RootNode(&self) -> Root<Node> { | ||
| match self.GetParentNode() { | ||
| Some(parent) => parent.RootNode(), |
This comment has been minimized.
This comment has been minimized.
aravind-pg
Apr 20, 2016
•
Contributor
In this case it seems easy enough to make this an explicit loop roughly as follows (since Rust doesn't actually have tail-call optimization):
let mut current = &self;
while let Some(ref parent) = current.GetParentNode() {
current = parent;
}
Root::from_ref(current)
This comment has been minimized.
This comment has been minimized.
dzbarsky
Apr 21, 2016
Member
you should do return self.inclusive_ancestors().last() or something like that
|
r? @Ms2ger (since you opened the original issue) |
|
I made the needed changes. |
| @@ -1858,6 +1858,12 @@ impl NodeMethods for Node { | |||
| } | |||
| } | |||
|
|
|||
| // https://dom.spec.whatwg.org/#dom-node-rootnode | |||
| fn RootNode(&self) -> Root<Node> { | |||
| let tree_root = self.inclusive_ancestors().last().unwrap(); | |||
This comment has been minimized.
This comment has been minimized.
|
Oops! fixed |
|
LGTM once you squash into a single commit. |
|
Squashed. |
|
@bors-servo r+ Thank you! |
|
|
Implement Node::rootNode Fixes #10747. I don't know whether it's OK to recurse up the tree, though it is a tail call. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10757) <!-- Reviewable:end -->
|
|
ineol commentedApr 20, 2016
Fixes #10747.
I don't know whether it's OK to recurse up the tree, though it is a tail call.
This change is