-
Notifications
You must be signed in to change notification settings - Fork 83
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
xml_child() function to make selection easier #94
Conversation
xml_children(x)[[search]] | ||
} else if (is.character(search)) { | ||
xml_find_first(x, xpath = paste0("./", search), ns = ns) | ||
} |
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.
Needs else
that throws error
|
6ca8f13
to
9b81903
Compare
#' | ||
#' @inheritParams xml_name | ||
#' @param only_elements For \code{xml_length}, should it count all children, | ||
#' or just children that are elements (the default)? | ||
#' @param search For \code{xml_child}, either the child number to return (by | ||
#' position), or the name of the child node to return. |
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.
"If there are multiple child nodes with the same name, the first will be returned"
9b81903
to
6d2e243
Compare
Current coverage is 58.89%@@ master #94 diff @@
==========================================
Files 30 30
Lines 1356 1355 -1
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 808 798 -10
- Misses 548 557 +9
Partials 0 0
|
6d2e243
to
85d75b7
Compare
@hadley anything else you think needs to change with this or should I merge it? |
LGTM |
With the current implementation you can do the following
Some questions before I add docs/tests
xml_sibling()
andxml_parent()
?xml_parent()
to use this same signature or keep it as-is?