r-lib / ansistrings

Manipulation of ANSI colored strings
Other
9 stars 5 forks source link

`ansi_strsplit` with perl arg "matched by multiple actual arguments" #4

Closed brodieG closed 7 years ago

brodieG commented 7 years ago
ansi_strsplit("abc,def", "\\W", perl=T)
# Error in gregexpr(pattern, text, perl = TRUE, ...) : 
#  formal argument "perl" matched by multiple actual arguments
brodieG commented 7 years ago

Actually, the problem is in rematch2::re_exec_all. "Fix" might be to just add a perl formal with default TRUE to that function, but I'm not sure if you default to perl for any specific reason (other than wouldn't it be nice if all R regular expressions were perl, cough regexec cough).

gaborcsardi commented 7 years ago

For capture groups, you need perl = TRUE and rematch2 works best for these. But yeah, adding a perl = TRUE somewhere is a good fix.

brodieG commented 7 years ago

So to get stuff to work with ansi_strwrap I took the liberty of adding this argument to rematch2. Seems like this is something that we can't just merge without thinking through the implications.

Here is my branch. All tests past FWIW, but the documentation is no longer technically correct since you can now pass perl=FALSE (though presumably that breaks stuff, or does it just fail to capture subgroups) so it's not just using PCRE as per the function titles.

Re capture groups, I they work with the (buggy) TRE implementation:

x <- "hello there how are you today sir"
regmatches(x, regexec("\\w+ (\\w+) \\w+", x, perl=F))
## [[1]]
## [1] "hello there how" "there"          

but that only does the first one. Also, to my astonishment regexec appears to now support perl. I think this is new.

gaborcsardi commented 7 years ago

That's good, can you pls submit a PR to rematch2?

brodieG commented 7 years ago

Will do, just need to clean few things up.

gaborcsardi commented 7 years ago

OK, so with the rematch2 change, this is fixed, right?

brodieG commented 7 years ago

Yes, fixed as far as I can tell.