-
Notifications
You must be signed in to change notification settings - Fork 746
Use iterators for comment children and attributes fix #166 #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
6a4ddeb
to
9e8bae7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest looks awesome, thanks for doing this!
You'll have to rebase over #158 I think (I didn't expect you to do it this fast!), but apart from that and the nits this is great, thanks!
unsafe { | ||
String_ { x: clang_HTMLStartTag_getAttrName(self.x, idx) }.to_string() | ||
/// An iterator for a comment's children | ||
pub struct CommentChildrenIntoIterator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Probably CommentChildrenIterator
? IntoIterator
usually means that it "consumes" the value, which doesn't make a lot of sense in this case.
} | ||
|
||
/// An iterator for a comment's attributes | ||
pub struct CommentAttributesIntoIterator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, CommentAttributesIterator
seems like a better name for this, since it doesn't consume the comment.
name: String_ { x: unsafe { clang_HTMLStartTag_getAttrName(self.x, idx) } }.to_string(), | ||
value: String_ { x: unsafe { clang_HTMLStartTag_getAttrValue(self.x, idx) } }.to_string() | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Rust style is } else {
.
9e8bae7
to
daabd07
Compare
Thanks for the review. |
☔ The latest upstream changes (presumably #158) made this pull request unmergeable. Please resolve the merge conflicts. |
daabd07
to
91eb86d
Compare
@bors-servo r+ Neat, thanks! |
📌 Commit 91eb86d has been approved by |
☀️ Test successful - status-travis |
Fix #166