From 0d181f6905392b0c543a490d6047613334404bad Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 19 Apr 2021 13:25:37 -0500 Subject: [PATCH] Assume git users can undo (#1424) * Assume git users can undo Fixes #1376 * Add an option * Change name; document in vignette * Apply suggestions from code review Co-authored-by: Jennifer (Jenny) Bryan * Improve `can_overwrite()` behaviour re: active project * Note this change in principles.md * Update NEWS and setup vignette to reflect implementation * Missing word Co-authored-by: Jennifer (Jenny) Bryan --- NEWS.md | 6 +++ R/utils.R | 12 +++++ principles.md | 12 +++-- tests/testthat/test-write.R | 69 ++++++++++++++++++++++++++-- vignettes/articles/usethis-setup.Rmd | 7 ++- 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/NEWS.md b/NEWS.md index 6bc64bc3b..d55ef8fb0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # usethis (development version) +* `"usethis.overwrite"` is a new option. When set to `TRUE`, usethis overwrites + an existing file without asking for user confirmation if the file is inside + a Git repo. The normal Git workflow makes it easy to see and selectively + accept/discard any proposed changes. This behaviour is strictly opt-in + (#1376). + * `use_lifecycle()` now imports `lifecycle::deprecated()` (#1419). * `use_code_of_conduct()` now requires a `contact` argument to supply contact diff --git a/R/utils.R b/R/utils.R index e00114d8b..2796a21f7 100644 --- a/R/utils.R +++ b/R/utils.R @@ -3,6 +3,18 @@ can_overwrite <- function(path) { return(TRUE) } + if (getOption("usethis.overwrite", FALSE)) { + # don't activate a project + # don't assume `path` is in the active project + if (is_in_proj(path) && uses_git()) { # path is in active project + return(TRUE) + } + if (possibly_in_proj(path) && # path is some other project + with_project(proj_find(path), uses_git(), quiet = TRUE)) { + return(TRUE) + } + } + if (is_interactive()) { ui_yeah("Overwrite pre-existing file {ui_path(path)}?") } else { diff --git a/principles.md b/principles.md index c1b2cf20c..1c0ae320f 100644 --- a/principles.md +++ b/principles.md @@ -58,13 +58,15 @@ Two opposing mindsets: - Everything should refer to the active project, unless there's a specific reason not to. Ideally, the exported file writing helpers would not make direct reference to the active project. -But we are violating this somewhat. +However, we violate this, with due care, when it benefits us: -`write_utf8()` potentially consults the project `path` lives in re: line ending. -So its implementation takes care to respect that, but also to not change the active project. +- `write_utf8()` potentially consults the project in which `path` lives in re: line ending. + So its implementation takes care to respect that, but also to not change the active project. -Likewise, `write_union()` uses the active project, if such exists, to create a humane path in its message. -It also actively avoids activating or changing the project. +- `write_over()` (`can_overwrite()`, really) does same, except in that case we're determining whether `path` is within a Git repo. + +- `write_union()` uses the active project, if such exists, to create a humane path in its message. + It also actively avoids activating or changing the project. Git/GitHub helpers generally assume we're working on the Git repo that is also the active project. These are unexported. diff --git a/tests/testthat/test-write.R b/tests/testthat/test-write.R index 2f4234f08..996fbe9b0 100644 --- a/tests/testthat/test-write.R +++ b/tests/testthat/test-write.R @@ -173,10 +173,71 @@ test_that("write_over() writes a de novo file", { expect_identical(read_utf8(tmp), letters[1:3]) }) -test_that("write_over() leaves file 'as is'", { - tmp <- file_temp() +test_that("write_over() leaves file 'as is' (outside of a project)", { + local_interactive(FALSE) + tmp <- withr::local_file(file_temp()) + writeLines(letters[1:3], tmp) + before <- read_utf8(tmp) - write_over(tmp, letters[1:3], quiet = TRUE) - expect_identical(before, read_utf8(tmp)) + write_over(tmp, letters[4:6], quiet = TRUE) + expect_identical(read_utf8(tmp), before) + + # usethis.overwrite shouldn't matter for a file outside of a project + withr::with_options( + list(usethis.overwrite = TRUE), + { + write_over(tmp, letters[4:6], quiet = TRUE) + expect_identical(read_utf8(tmp), before) + } + ) +}) + +test_that("write_over() works in active project", { + local_interactive(FALSE) + create_local_project() + + tmp <- proj_path("foo.txt") + writeLines(letters[1:3], tmp) + + before <- read_utf8(tmp) + write_over(tmp, letters[4:6], quiet = TRUE) + expect_identical(read_utf8(tmp), before) + + use_git() + withr::with_options( + list(usethis.overwrite = TRUE), + { + write_over(tmp, letters[4:6], quiet = TRUE) + expect_identical(read_utf8(tmp), letters[4:6]) + } + ) +}) + +test_that("write_over() works for a file in a project that is not active", { + local_interactive(FALSE) + owd <- getwd() + proj <- create_local_project() + use_git() + + tmp <- proj_path("foo.txt") + writeLines(letters[1:3], tmp) + + withr::local_dir(owd) + local_project(NULL) + expect_false(proj_active()) + + tmp <- path(proj, "foo.txt") + before <- read_utf8(tmp) + write_over(tmp, letters[4:6], quiet = TRUE) + expect_identical(read_utf8(tmp), before) + + withr::with_options( + list(usethis.overwrite = TRUE), + { + write_over(tmp, letters[4:6], quiet = TRUE) + expect_identical(read_utf8(tmp), letters[4:6]) + } + ) + expect_false(proj_active()) }) diff --git a/vignettes/articles/usethis-setup.Rmd b/vignettes/articles/usethis-setup.Rmd index b6ee09fb6..7ce330fee 100644 --- a/vignettes/articles/usethis-setup.Rmd +++ b/vignettes/articles/usethis-setup.Rmd @@ -70,6 +70,10 @@ Certain options are consulted by usethis and allow you to set personal defaults: console. * `usethis.destdir`: a default directory to use in `create_from_github()` and `use_course()`. + * `usethis.overwrite`: if `TRUE`, usethis overwrites an existing file without + asking for user confirmation if the file is inside a Git repo. The rationale + is that the normal Git workflow makes it easy to see and selectively + accept/discard any proposed changes. Define any of these options in your `.Rprofile`, which can be opened for editing via `usethis::edit_r_profile()`. Here is example code: @@ -86,7 +90,8 @@ options( ), Version = "0.0.0.9000" ), - usethis.destdir = "~/the/place/where/I/keep/my/R/projects" + usethis.destdir = "~/the/place/where/I/keep/my/R/projects", + usethis.overwrite = TRUE ) ```