Check exclusive #375
Check exclusive #375
Conversation
| if (!missing(css) && !missing(xpath)) | ||
| stop("Please supply css or xpath, not both", call. = FALSE) | ||
| make_selector <- function(css, xpath, call = rlang::caller_env()) { | ||
| switch( |
There was a problem hiding this comment.
I'd suggest keeping the basic flow of the function the same, just calling check_exclusive() (without the rlang::) first. I'd suggest you leave the string checking messages as is in this PR, as those would be better handled with the standalone rlang input checkers.
| @@ -78,30 +78,32 @@ html_elements <- function(x, css, xpath) { | |||
|
|
|||
| #' @export | |||
| html_elements.default <- function(x, css, xpath) { | |||
| xml2::xml_find_all(x, make_selector(css, xpath)) | |||
| xpath <- make_selector(css, xpath) | |||
There was a problem hiding this comment.
I think these functions can stay the same too.
|
I can finish it off in a day or two! I'll need to remember why I made those changes you called out to see if I want to argue for them; right now I can't remember, but I assume I had an argument for them at the time. I'll likely have more questions about what you want when I get a chance to dig into this! |
|
Should I update the workflow to match something more recently updated (eg httr2) while I'm at it, to kill those |
|
I don't see the failures on main, so you should just be able to rebase/merge from main. |
|
Actually, I just started converting the error to use cli, so it might be just as easy for me to do |
|
Ok, I won't be offended if you go ahead and clear this out without me! I'll be happy to review, and I'll still try to figure out why I felt the need to change other things! |
|
Done in #386. I'm sure there's still plenty of room for improvement though. |
Closes #374
I updated all other errors in
make_selector()to what I believe to be the current preferred style, but did not update everything infind_href()in case I'm wrong about the preference.