-
Notifications
You must be signed in to change notification settings - Fork 286
Refactor get_active_file() #1730
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
Conversation
* Make it much broader so it now works with any file in `R/`, `src/` or `tests/testthat`. Fixes #1566 * This new foundation made it possible to make `use_c()` and `use_rcpp()` (and friends) to work the same way as `use_r()`/use_test()`. * Since I was in here I fixed #1690. * I also refactored the code to make it easier to test. * I switched the errors I touch to use `cli::cli_abort()` so the messaging can point to the correct user facing function. * I generally refactored the error handling so it happens in one central location.
990cbad
to
047cb88
Compare
#' @inheritParams edit_file | ||
#' @seealso The [testing](https://r-pkgs.org/tests.html) and | ||
#' [R code](https://r-pkgs.org/r.html) chapters of | ||
#' [R Packages](https://r-pkgs.org). | ||
#' @export | ||
use_r <- function(name = NULL, open = rlang::is_interactive()) { | ||
check_not_empty_file_name(name) |
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.
I inlined check_not_empty_filename()
into check_file_name()
and moved that into compute_name()
.
|
||
test_path <- proj_path("tests", "testthat", paste0("test-", name, ".R")) | ||
if (!file_exists(test_path)) { | ||
ui_todo("Call {ui_code('use_test()')} to create a matching test file") |
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.
I removed this because no other helper includes similar text, but I can restore it if you think it's important.
#' | ||
#' @details | ||
#' | ||
#' When using compiled code, please note that there must be at least one file |
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.
This weird wrinkle (implemented in use_src_example_script())
is no longer needed because we always create a file.
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.
I didn't do any manual testing, but I assume you have.
R/r.R
Outdated
valid_file_name <- function(x) { | ||
grepl("^[a-zA-Z0-9._-]+$", x) | ||
} | ||
rel_path <- path_dir(proj_rel_path(path)) |
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.
rel_path
feels like a confusing name for the immediate parent directory within the project, but I guess it's not terribly important.
Co-authored-by: Jennifer (Jenny) Bryan <jenny.f.bryan@gmail.com>
R/
,src/
ortests/testthat
. Fixes Teachuse_test()
/test_active_file()
aboutsrc
#1566use_c()
anduse_rcpp()
(and friends) to work the same way asuse_r()
/use_test()`.cli::cli_abort()
so the messaging can point to the correct user facing function.