paleolimbot commented on code in PR #36784:
URL: https://github.com/apache/arrow/pull/36784#discussion_r1269414344
##########
r/R/dplyr-funcs-string.R:
##########
@@ -57,12 +57,14 @@ get_stringr_pattern_options <- function(pattern) {
}
}
ensure_opts <- function(opts) {
+
if (is.character(opts)) {
opts <- list(pattern = opts, fixed = FALSE, ignore_case = FALSE)
}
opts
}
- ensure_opts(eval(pattern))
+
+ ensure_opts(eval_tidy(pattern, env =
new_environment(as.list(rlang::current_env()), parent = caller_env())))
Review Comment:
Here, I believe `pattern` is `quote()`d expression (created using `enexpr()`
in one or more functions that call this one). The environment that is required
is the calling environment of `str_xxxx()` (i.e., what `rlang::caller_env()`
would return if it was called directly from `str_xxxx()`). Rather than keep
track of that environment ourselves, I *think* we can use rlang's "quosure"
(which is an expression AND and environment) by:
- Instead of `enexpr()` use `enquo()`
- Instead of `eval()` use `eval_tidy(pattern, mask = list(regex = regex,
coll = coll, boundary = boundary))`
The alternative would be to add an `envir` argument to
`get_stringr_pattern_options()` and any function that calls it. Given that the
documentation for `enexpr()` warns not to use it for this exact reason, I think
the `enquo()`/`eval_tidy()` solution is probably best.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
