Open dmi3kno opened 5 years ago
Hi @dmi3kno I'm going to try and summarize just to make sure I understand this all.
Regarding the challenges:
No function to express individual word characters ([[:alnum:]]
+ _
). Solution is to add rx_word_chr() %>% rx_one_or_more()
combo (or add a rep
argument to everything). The code chunk with rx_char()
, that's actually referring to rx_word_char()
right? It should be because rx_char()
sounds misleading if it's \\w
under the hood (i.e. !
is a character but wont be matched).
rx_count()
accepts ranges, nice addition 👍
rx_word_edge()
need word boundaries. Same as in point 1, I assume the code chunk just wasn't edited with the name change. Word boundaries will be expressed as rx_word_edge()
. There is also \\B
(not word edge), should we support !rx_word_edge()
?
The biggest problem is groups of characters, double sanitization becomes an issue if you want to use rx_
inside an rx_
call. Solution is create rx_group()
, identical to rx_any_of()
but doesn't sanitize. Side note, rx_any_of()
might better be represented as [whatever]*
. Additionally, this function might better be named rx_one_of()
since it matches one of the characters in the set (i.e. gr[ae]y).
Regarding the solution:
rx()
is redundant but I couldn't get away from needing to pass value parameter at the start so rx
was the quick and dirty solution. So more than happy to find a more elegant solution to this. Is it the S3 class you mention or the ...
or both?
rx_one_or_more()
isn't very clear in the nested pipes example. Using the example from your pull request, am I on the right path with this translation:
# old
x <- rx_word_edge() %>%
rx_alpha() %>%
rx_one_or_more() %>%
rx_word_edge()
# new
x <- rx_word_edge() %>%
rx_alpha(rep = "any") %>%
rx_word_edge()
Yes, rx_char()
(or better yet, rx_literal()
as you mention) implies things other than word characters so without an argument it should be rx_word_char()
given that this would return \\w
.
If rx_char()
behaves like I think it does in the example, rx_literal()
sounds most fitting to me. rx_literal("@")
literally gives you @ and nothing more.
Letting the user know something is going to be sanitized sounds good but might use different words like "special characters will be escaped" or something, don't know if that's clearer to someone (including myself 😅) without much regex knowledge.
I do not like nested pipes, I would prefer to avoid that! The second solution looks much cleaner.
With the latest version of RVerbalExpressions
and some of the functions you wrote, the closest I can get without using the rep
argument is:
library(RVerbalExpressions)
rx_word_char <- function(.data = NULL, value = NULL) {
if(missing(value))
return(paste0(.data, "\\w"))
paste0(.data, sanitize(value))
}
rx_group <- function(.data = NULL, value) {
paste0(.data, "[", value, "]")
}
rx_any_of <- function(.data = NULL, value, ...) {
if(missing(...))
return(paste0(.data, "[", sanitize(value), "]"))
paste0(.data, "[", value, sanitize(...), "]")
}
rx_literal <- function(.data = NULL, value) {
paste0(.data, value)
}
x <- rx_word_edge() %>%
rx_any_of(rx_word_char(), ".%+-") %>%
rx_one_or_more() %>%
rx_literal("@") %>%
rx_any_of(rx_word_char(), ".-") %>%
rx_one_or_more() %>%
rx_word_char(".") %>%
rx_alpha() %>%
rx_count(n = 2:6) %>%
rx_word_edge()
txt <- "This text contains email first.last@gmail.com and noname@post.io. The latter is no longer valid."
stringr::str_extract_all(txt, x)[[1]]
#> [1] "first.last@gmail.com" "noname@post.io"
Looking at that long pipe makes the rep
argument worth it to me. This would avoid 3 lines (lines 3, 6, and 9).
Sorry for messy post. I was writing it and contributing new functions at the same time, so it reflects my own evolution of thinking. I will be more consistent going forward.
!rx_word_edge()
. I have never implemented overloading of !
. On the first look it seems to require custom class, which is what I believe we should do anyways. But I also cant recall seeing !
with the pipe. How would that work?rx_one_of()
:+1:. I am on it!rep
argument: :100:. rep
seems like modifier to me (similar to rx_count
, so that rep=any
(quantifier *
) is rx_count(c(0, NA))
or {0,}
and rep=some
(quantifier +
) is the same as rx_count(c(1, NA))
or {1,}
). If we go down this route, rep
argument could be recepticle for both wildcard quantifiers and counts and it is not unthinkable to express:
# the following is equivalent to `[a-zA-Z]*?`
rx_alpha(rep="any", mode="lazy")
rx_one_or_more()
and rx_none_or_more()
as well as implement more concise rep
interface.rx_word_char(.data=NULL)
shall only mean \\w
. Having said that, I find rx_word_char
slightly confusing. I actually find the whole \\w
confusing and remember looking up multiple times what characters are included. Imagine the world where we have rx_alphanum()
and rx_alpha_num()
with the latter also including _
. (Or maybe rx_alnum()
and rx_alnum_()
, although people might confuse trailing _
with standard evaluation, thank you dplyr
).rx_literal()
:+1: I am on it !
rx_group()
in my initial post, I don't like it. As I said, nested pipes are confusing. So let's go down the route of ...
and parsing content class-dependent. I see you suggested rx_any_of (.data, value, ...)
, but that's not what I am talking about.
rx_one_of <- function(.data = NULL, ... ) {
args <- sapply(list(...), function(x) if(inherits(x, "rx_string")) x else sanitize(x))
args_str <- Reduce(paste0, args)
paste0(.data, "[", args_str, "]")
}
This would require custom class to be output by every of our functions:
rx_word_char <- function(.data = NULL) {
res <- paste0(.data, "\\w")
class(res) <- unique(c("rx_string", class(res))) # to avoid accidental double "classing"
res
}
rx_literal <- function(.data=NULL, value) {
res <- paste0(.data, sanitize(value))
class(res) <- unique(c("rx_string", class(res))) # to avoid accidental double "classing"
res
}
But then you can do things like:
```r
rx() %>%
rx_one_of(rx_word_char(), rx_literal(value="?"), "abc")
#> [1] "[\\w\\?abc]"
The only thing I feel we should watch out for, is that by adding all of these modifying arguments we are on the way back to complexity and away from intuitive interface. So I say we keep both rx_one_or_more() and rx_none_or_more() as well as implement more concise rep interface.
100% agree, I would rather have an intuitive API that does less rather than a somewhat clunky API that can do a whole lot. Given the number of functions that have been added, I wonder if a vignette covering common regex use cases and which functions to use would be helpful?
!
with the pipe is something I've never seen either and I just realized that once you mentioned it. If it is possible, most likely way beyond me.I like rep
, it should be there and I agree that rx_one_or_more
and rx_none_or_more
stay. I usually don't like the idea of aliases or multiple functions that provide the same functionality but for the sake of keeping the functions from the original JS repo and having a more verbose option, it is worth while to keep.
Now that you mentioned, rx_word_char
is a little confusing, it's basically rx_alnum
+ _. I think rx_alpha_num()
sounds the best. It might not immediately be clear what is does but if the docs quickly express alphabet + underscore + numbers I think it should be clear, easy to remember.
The last part using the ellipses:
rx_one_of <- function(.data = NULL, ... ) {
args <- sapply(list(...), function(x) if(inherits(x, "rx_string")) x else sanitize(x))
args_str <- Reduce(paste0, args)
paste0(.data, "[", args_str, "]")
}
Looks great, I haven't done much or any programming using ellipses but this looks much more elegant! Very excited about this.
To do here:
rx_literal
rx_string
. Make a class constructor new_rx
(unexported). Implement crucial methods for vectorized class (ref. Hadley)rep
argument to most(?) functions. Options (integer vector/"any"/"some").mode
argumentrx_none_of
as inverse of rx_one_of
. This might be overlapping rx_anything_but
rx_alnum
renamed to rx_alphanum
, and rx_word_char
to rx_alpha_num
rx_one_of
with ...
. Deprecate rx_any_of
rx_find
with ...
Problem
I think the package will be incomplete until we find a way to express groups of characters. Here's a challenge to express email pattern matching in
rx
:Challenges
First of all, I dont know of the way to express single "word" character (
alnum
+_
). We usedrx_word
to denote\\w+
and perhaps it should have beenrx_word_char() %>% rx_one_or_more()
.I also extended
rx_count
to cases of ranges of inputFinally, we dont have a way to express word boundaries (
\\b
) and it might be useful to denote them. We shall call this functionrx_word_edge
Finally, our biggest problem is that there's no way to express groups of characters, other than through
rx_any_of()
, but if we pass otherrx
expressions, values will be sanitized twice, meaning that we will get four backslashes before each symbol instead of two.Solution
Here's what it looks like when we put all pieces together:
The code works but I don't like it.
rx
look redundant (I believe, there's a way to get rid of it entirely using specialized class, see below).rx_one_or_more()
is referring to. I wonder if all functions should haverep
argument with default optionone
and optionssome
/any
in addition to whatrx_count
does today.rx_char()
without arguments be calledrx_wordchar
?rx_char()
with arguments be calledrx_literal()
orrx_plain
?rx_group
is artificial construct, a duplicate ofrx_any_of
, but without sanitization. Here I see couple of solutions. a. Allow "nested pipes" (as I have done above). Create S3 class and this way detect when type ofvalue
argument is not character, butrx_string
. Input of this class do not need to be sanitized, because it has been sanitized at creation. b. Do not allow "nested pipes". Instead definerx_any_of()
to have...
and allow multiple arguments mixing functions and characters. Then hypotherical pipe would look like this:It's a lot to digest, but somehow everything related to one particular problem. Happy to split the issue once we identify the issues worth tackling.