From 4ce704bf2cb35b873c673a4673d99a97c1e120b4 Mon Sep 17 00:00:00 2001 From: laurabrianna Date: Thu, 15 Aug 2024 15:10:53 -0700 Subject: [PATCH 1/7] Bump default serialization version in use_data() to 3 (#2044) * Fixes #1966 - bumped default serialization version in use_data() to 3 * addition of bullet in NEWS.md to alert for update to use_data() version bump * update param in use_data & related test for default version 3 --- NEWS.md | 1 + R/data.R | 8 ++++---- man/use_data.Rd | 8 ++++---- tests/testthat/test-data.R | 6 +++--- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8c43da1f2..b06bd788c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # usethis (development version) +* `use_data()` now uses serialization version 3 by default. (@laurabrianna, #2044) * Reverse dependency checks are only suggested if they exist (#1817, @seankross). diff --git a/R/data.R b/R/data.R index cc407d745..e19cb04b7 100644 --- a/R/data.R +++ b/R/data.R @@ -18,9 +18,9 @@ #' files. If you really want to do so, set this to `TRUE`. #' @param compress Choose the type of compression used by [save()]. #' Should be one of "gzip", "bzip2", or "xz". -#' @param version The serialization format version to use. The default, 2, was -#' the default format from R 1.4.0 to 3.5.3. Version 3 became the default from -#' R 3.6.0 and can only be read by R versions 3.5.0 and higher. +#' @param version The serialization format version to use. The default, 3, can +#' only be read by R versions 3.5.0 and higher. For R 1.4.0 to 3.5.3, use +#' version 2. #' @inheritParams base::save #' #' @seealso The [data chapter](https://r-pkgs.org/data.html) of [R @@ -38,7 +38,7 @@ use_data <- function(..., internal = FALSE, overwrite = FALSE, compress = "bzip2", - version = 2, + version = 3, ascii = FALSE) { check_is_package("use_data()") diff --git a/man/use_data.Rd b/man/use_data.Rd index c4a0b93db..5f5fc1f39 100644 --- a/man/use_data.Rd +++ b/man/use_data.Rd @@ -10,7 +10,7 @@ use_data( internal = FALSE, overwrite = FALSE, compress = "bzip2", - version = 2, + version = 3, ascii = FALSE ) @@ -35,9 +35,9 @@ files. If you really want to do so, set this to \code{TRUE}.} \item{compress}{Choose the type of compression used by \code{\link[=save]{save()}}. Should be one of "gzip", "bzip2", or "xz".} -\item{version}{The serialization format version to use. The default, 2, was -the default format from R 1.4.0 to 3.5.3. Version 3 became the default from -R 3.6.0 and can only be read by R versions 3.5.0 and higher.} +\item{version}{The serialization format version to use. The default, 3, can +only be read by R versions 3.5.0 and higher. For R 1.4.0 to 3.5.3, use +version 2.} \item{ascii}{if \code{TRUE}, an ASCII representation of the data is written. The default value of \code{ascii} is \code{FALSE} which diff --git a/tests/testthat/test-data.R b/tests/testthat/test-data.R index dfe2b017d..f2f893aee 100644 --- a/tests/testthat/test-data.R +++ b/tests/testthat/test-data.R @@ -62,14 +62,14 @@ test_that("use_data() honors `overwrite` for internal data", { expect_identical(letters2, rev(letters)) }) -test_that("use_data() writes version 2 by default", { +test_that("use_data() writes version 3 by default", { create_local_package() x <- letters - use_data(x, internal = TRUE, version = 2, compress = FALSE) + use_data(x, internal = TRUE, compress = FALSE) expect_identical( rawToChar(readBin(proj_path("R", "sysdata.rda"), n = 4, what = "raw")), - "RDX2" + "RDX3" ) }) From a225a21f3e336394051584411efb4d351aa9d78a Mon Sep 17 00:00:00 2001 From: "J.P. Le Cavalier" Date: Thu, 15 Aug 2024 16:49:09 -0700 Subject: [PATCH 2/7] Let `use_package()` decrement a package dependency. (#2043) * Let `use_package()` decrement a package dependency. Fixes #1957. * Using if () ... else () instead of ifelse() on non-vectorized operation * Style * Simplify glue string * Credit @jplecavalier for the PR --------- Co-authored-by: Jenny Bryan --- NEWS.md | 7 +++++++ R/helpers.R | 27 +++++++++++++++------------ tests/testthat/_snaps/helpers.md | 7 +++++++ tests/testthat/test-helpers.R | 4 +++- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/NEWS.md b/NEWS.md index b06bd788c..311e2707d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,13 @@ # usethis (development version) +## Bug fixes and minor improvements + +* `use_package()` now decreases a package minimum version required when + `min_version` is lower than what is currently specified in the DESCRIPTION + file (@jplecavalier, #1957). + * `use_data()` now uses serialization version 3 by default. (@laurabrianna, #2044) + * Reverse dependency checks are only suggested if they exist (#1817, @seankross). diff --git a/R/helpers.R b/R/helpers.R index edbdbdd87..d09d28278 100644 --- a/R/helpers.R +++ b/R/helpers.R @@ -62,19 +62,21 @@ use_dependency <- function(package, type, min_version = NULL) { "!" = "Package {.pkg {package}} is already listed in {.field {existing_type}} in DESCRIPTION; no change made." )) - } else if (delta == 0 && !is.null(min_version)) { - # change version - upgrade <- existing_version == "*" || - numeric_version(min_version) > version_spec(existing_version) - if (upgrade) { - ui_bullets(c( - "v" = "Increasing {.pkg {package}} version to {.val {version}} in - DESCRIPTION." - )) - desc$set_dep(package, type, version = version) - desc$write() - changed <- TRUE + } else if (delta == 0 && version_spec(version) != version_spec(existing_version)) { + if (version_spec(version) > version_spec(existing_version)) { + direction <- "Increasing" + } else { + direction <- "Decreasing" } + + ui_bullets(c( + "v" = "{direction} {.pkg {package}} version to {.val {version}} in + DESCRIPTION." + )) + desc$set_dep(package, type, version = version) + desc$write() + changed <- TRUE + } else if (delta > 0) { # moving from, e.g., Suggests to Imports ui_bullets(c( @@ -96,6 +98,7 @@ r_version <- function() { } version_spec <- function(x) { + if (x == "*") x <- "0" x <- gsub("(<=|<|>=|>|==)\\s*", "", x) numeric_version(x) } diff --git a/tests/testthat/_snaps/helpers.md b/tests/testthat/_snaps/helpers.md index 807c258c3..d5fc01a0b 100644 --- a/tests/testthat/_snaps/helpers.md +++ b/tests/testthat/_snaps/helpers.md @@ -26,6 +26,13 @@ Message v Increasing crayon version to ">= 2.0.0" in DESCRIPTION. +--- + + Code + use_dependency("crayon", "Imports", min_version = "1.0.0") + Message + v Decreasing crayon version to ">= 1.0.0" in DESCRIPTION. + # use_dependency() upgrades a dependency Code diff --git a/tests/testthat/test-helpers.R b/tests/testthat/test-helpers.R index 3b063325d..d408b77fd 100644 --- a/tests/testthat/test-helpers.R +++ b/tests/testthat/test-helpers.R @@ -52,7 +52,9 @@ test_that("we message for version change and are silent for same version", { expect_snapshot( use_dependency("crayon", "Imports", min_version = "2.0.0") ) - expect_silent(use_dependency("crayon", "Imports", min_version = "1.0.0")) + expect_snapshot( + use_dependency("crayon", "Imports", min_version = "1.0.0") + ) }) ## https://github.com/r-lib/usethis/issues/99 From 9aa221bc0ea50b4119e312398ec85b3496285895 Mon Sep 17 00:00:00 2001 From: Etienne Bacher <52219252+etiennebacher@users.noreply.github.com> Date: Sat, 24 Aug 2024 01:15:53 +0200 Subject: [PATCH 3/7] Fix some lints (#2048) * Fix some lints * revert expect_named --- R/data.R | 2 +- R/proj-desc.R | 2 +- R/use_standalone.R | 2 +- R/utils-git.R | 2 +- R/utils-github.R | 2 +- tests/manual/manual-use-github.R | 2 +- tests/testthat/test-proj.R | 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/R/data.R b/R/data.R index e19cb04b7..5003c0c77 100644 --- a/R/data.R +++ b/R/data.R @@ -92,7 +92,7 @@ get_objs_from_dots <- function(.dots) { } is_name <- vapply(.dots, is.symbol, logical(1)) - if (any(!is_name)) { + if (!all(is_name)) { ui_abort("Can only save existing named objects.") } diff --git a/R/proj-desc.R b/R/proj-desc.R index d2e110a7e..aa962b5c4 100644 --- a/R/proj-desc.R +++ b/R/proj-desc.R @@ -41,7 +41,7 @@ proj_desc_field_update <- function(key, value, overwrite = TRUE, append = FALSE) return(invisible()) } - if (!overwrite && length(old > 0) && any(old != "")) { + if (!overwrite && length(old) > 0 && any(old != "")) { ui_abort(" {.field {key}} has a different value in DESCRIPTION. Use {.code overwrite = TRUE} to overwrite.") diff --git a/R/use_standalone.R b/R/use_standalone.R index e23d1cba7..11ca47e12 100644 --- a/R/use_standalone.R +++ b/R/use_standalone.R @@ -247,7 +247,7 @@ as_version_info_row <- function(field, error_call = caller_env()) { ver <- strsplit(ver, " ")[[1]] - if (!is_character(ver, n = 2) || any(is.na(ver)) || !all(nzchar(ver))) { + if (!is_character(ver, n = 2) || anyNA(ver) || !all(nzchar(ver))) { cli::cli_abort( c( "Can't parse version `{field}` in `imports:` field.", diff --git a/R/utils-git.R b/R/utils-git.R index 810988d45..9a3a57cb5 100644 --- a/R/utils-git.R +++ b/R/utils-git.R @@ -83,7 +83,7 @@ git_user_get <- function(where = c("de_facto", "local", "global")) { # translate from "usethis" terminology to "git" terminology where_from_scope <- function(scope = c("user", "project")) { - scope = match.arg(scope) + scope <- match.arg(scope) where_scope <- c(user = "global", project = "de_facto") diff --git a/R/utils-github.R b/R/utils-github.R index 66fca74bd..abba6d408 100644 --- a/R/utils-github.R +++ b/R/utils-github.R @@ -193,7 +193,7 @@ github_remotes <- function(these = c("origin", "upstream"), # 1. Did we call the GitHub API? Means we know `is_fork` and the parent repo. # 2. If so, did we call it with auth? Means we know if we can push. grl$github_got <- map_lgl(repo_info, ~ length(.x) > 0) - if (isTRUE(github_get) && any(!grl$github_got)) { + if (isTRUE(github_get) && !all(grl$github_got)) { oops <- which(!grl$github_got) oops_remotes <- grl$remote[oops] oops_hosts <- unique(grl$host[oops]) diff --git a/tests/manual/manual-use-github.R b/tests/manual/manual-use-github.R index aefde5e88..c98f02860 100644 --- a/tests/manual/manual-use-github.R +++ b/tests/manual/manual-use-github.R @@ -59,7 +59,7 @@ expect_match(desc::desc_get_field("BugReports"), BugReports) # remove the GitHub links desc::desc_del(c("BugReports", "URL")) -expect_true(all(!desc::desc_has_fields(c("BugReports", "URL")))) +expect_true(!any(desc::desc_has_fields(c("BugReports", "URL")))) # restore the GitHub links # should see a warning that `host` is deprecated and ignored diff --git a/tests/testthat/test-proj.R b/tests/testthat/test-proj.R index c2bf4a20c..826e578b1 100644 --- a/tests/testthat/test-proj.R +++ b/tests/testthat/test-proj.R @@ -148,12 +148,12 @@ test_that("with_project() runs code in temp proj, restores (lack of) proj", { ) proj_set_(NULL) - expect_identical(proj_get_(), NULL) + expect_null(proj_get_()) res <- with_project(path = temp_proj, proj_get_()) expect_identical(res, temp_proj) - expect_identical(proj_get_(), NULL) + expect_null(proj_get_()) }) test_that("with_project() runs code in temp proj, restores original proj", { From 27f735068815f93d7dadea3d7146ff81426e5661 Mon Sep 17 00:00:00 2001 From: Cynthia Huang <29718979+cynthiahqy@users.noreply.github.com> Date: Fri, 23 Aug 2024 16:18:30 -0700 Subject: [PATCH 4/7] Adds link to cli vignette on transition from ui_*() (#2047) * Adds link to cli vignette on transition from ui_*() Replaces "(LINK TO COME)" text in ui-legacy.R roxygen * Add link to cli conversion article in ui_* docs --- R/ui-legacy.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/ui-legacy.R b/R/ui-legacy.R index 92473119e..420a57037 100644 --- a/R/ui-legacy.R +++ b/R/ui-legacy.R @@ -13,8 +13,8 @@ #' #' usethis itself now uses cli internally for its UI, but these new functions #' are not exported and presumably never will be. There is a developer-focused -#' article on the process of transitioning usethis's own UI to use cli (LINK -#' TO COME). +#' article on the process of transitioning usethis's own UI to use cli: +#' [Converting usethis's UI to use cli](https://usethis.r-lib.org/articles/ui-cli-conversion.html) #' @details #' From 2cc9e5a4e5e3034399fafb65d871eec352d83a15 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Fri, 23 Aug 2024 16:20:29 -0700 Subject: [PATCH 5/7] Add period and re-document() --- R/ui-legacy.R | 2 +- man/ui-legacy-functions.Rd | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/R/ui-legacy.R b/R/ui-legacy.R index 420a57037..58edd5eca 100644 --- a/R/ui-legacy.R +++ b/R/ui-legacy.R @@ -14,7 +14,7 @@ #' usethis itself now uses cli internally for its UI, but these new functions #' are not exported and presumably never will be. There is a developer-focused #' article on the process of transitioning usethis's own UI to use cli: -#' [Converting usethis's UI to use cli](https://usethis.r-lib.org/articles/ui-cli-conversion.html) +#' [Converting usethis's UI to use cli](https://usethis.r-lib.org/articles/ui-cli-conversion.html). #' @details #' diff --git a/man/ui-legacy-functions.Rd b/man/ui-legacy-functions.Rd index 1a1976122..d1df6d888 100644 --- a/man/ui-legacy-functions.Rd +++ b/man/ui-legacy-functions.Rd @@ -75,8 +75,8 @@ make this transition: \code{vignette("usethis-ui", package = "cli")}. usethis itself now uses cli internally for its UI, but these new functions are not exported and presumably never will be. There is a developer-focused -article on the process of transitioning usethis's own UI to use cli (LINK -TO COME). +article on the process of transitioning usethis's own UI to use cli: +\href{https://usethis.r-lib.org/articles/ui-cli-conversion.html}{Converting usethis's UI to use cli}. } \details{ The \code{ui_} functions can be broken down into four main categories: From cec49b67f4e6c87c5e59d81ccba970e8ef4c1bf5 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 2 Oct 2024 08:14:55 -0500 Subject: [PATCH 6/7] Where possible, use existing github config to determine default branch (#2062) Fixes #2021 --- R/git-default-branch.R | 60 +++++++++++------------------------ R/github.R | 2 +- R/pr.R | 33 ++++++++++--------- tests/testthat/helper-mocks.R | 2 +- 4 files changed, 36 insertions(+), 61 deletions(-) diff --git a/R/git-default-branch.R b/R/git-default-branch.R index fdcaac79d..0154f4e6b 100644 --- a/R/git-default-branch.R +++ b/R/git-default-branch.R @@ -93,39 +93,18 @@ NULL #' git_default_branch() #' } git_default_branch <- function() { - repo <- git_repo() + git_default_branch_(github_remote_config()) +} - # TODO: often when we call git_default_branch(), we already have a GitHub - # configuration or target repo, as produced by github_remote_config() or - # target_repo(). In that case, we don't need to start from scratch as we do - # here. But I'm not sure it's worth adding complexity to allow passing this - # data in. - - # TODO: this critique feels somewhat mis-placed, i.e. it brings up a general - # concern about a repo's config (or the user's permissions and creds) - # related to whether github_remotes() should be as silent as it is about - # 404s - critique_remote <- function(remote) { - if (remote$is_configured && is.na(remote$default_branch)) { - ui_bullets(c( - "x" = "The {.val {remote$name}} remote is configured, but we can't - determine its default branch.", - " " = "Possible reasons:", - "*" = "The remote repo no longer exists, suggesting the local remote - should be deleted.", - "*" = "We are offline or that specific Git server is down.", - "*" = "You don't have the necessary permission or something is wrong - with your credentials." - )) - } - } +# If config is available, we can use it to avoid an additional lookup +# on the GitHub API +git_default_branch_ <- function(cfg) { + repo <- git_repo() - upstream <- git_default_branch_remote("upstream") + upstream <- git_default_branch_remote(cfg, "upstream") if (is.na(upstream$default_branch)) { - critique_remote(upstream) - origin <- git_default_branch_remote("origin") + origin <- git_default_branch_remote(cfg, "origin") if (is.na(origin$default_branch)) { - critique_remote(origin) db_source <- list() } else { db_source <- origin @@ -186,7 +165,7 @@ git_default_branch <- function() { # returns a whole data structure, because the caller needs the surrounding # context to produce a helpful error message -git_default_branch_remote <- function(remote = "origin") { +git_default_branch_remote <- function(cfg, remote = "origin") { repo <- git_repo() out <- list( name = remote, @@ -196,25 +175,22 @@ git_default_branch_remote <- function(remote = "origin") { default_branch = NA_character_ ) - url <- git_remotes()[[remote]] - if (length(url) == 0) { + cfg_remote <- cfg[[remote]] + if (!cfg_remote$is_configured) { out$is_configured <- FALSE return(out) } + out$is_configured <- TRUE - out$url <- url - - # TODO: generalize here for GHE hosts that don't include 'github' - parsed <- parse_github_remotes(url) - # if the protocol is ssh, I suppose we can't assume a PAT, i.e. it's better - # to use the Git approach vs. the GitHub API approach - if (grepl("github", parsed$host) && parsed$protocol == "https") { - remote_dat <- github_remotes(remote, github_get = NA) - out$repo_spec <- remote_dat$repo_spec - out$default_branch <- remote_dat$default_branch + out$url <- cfg_remote$url + + if (!is.na(cfg_remote$default_branch)) { + out$repo_spec <- cfg_remote$repo_spec + out$default_branch <- cfg_remote$default_branch return(out) } + # Fall back to pure git based approach out$default_branch <- tryCatch( { gert::git_fetch(remote = remote, repo = repo, verbose = FALSE) diff --git a/R/github.R b/R/github.R index c3eb0affb..34cc67392 100644 --- a/R/github.R +++ b/R/github.R @@ -61,7 +61,7 @@ use_github <- function(organisation = NULL, visibility <- match.arg(visibility) check_protocol(protocol) check_uses_git() - default_branch <- git_default_branch() + default_branch <- guess_local_default_branch() check_current_branch( is = default_branch, # glue-ing happens inside check_current_branch(), where `gb` gives the diff --git a/R/pr.R b/R/pr.R index c486d37b7..5d71233b8 100644 --- a/R/pr.R +++ b/R/pr.R @@ -184,7 +184,7 @@ pr_init <- function(branch) { } } - default_branch <- git_default_branch() + default_branch <- git_default_branch_(cfg) challenge_non_default_branch( "Are you sure you want to create a PR branch based on a non-default branch?", default_branch = default_branch @@ -237,7 +237,7 @@ pr_resume <- function(branch = NULL) { ui_bullets(c( "i" = "No branch specified ... looking up local branches and associated PRs." )) - default_branch <- git_default_branch() + default_branch <- guess_local_default_branch() branch <- choose_branch(exclude = default_branch) if (is.null(branch)) { ui_bullets(c("x" = "Repo doesn't seem to have any non-default branches.")) @@ -375,7 +375,7 @@ pr_push <- function() { repo <- git_repo() cfg <- github_remote_config(github_get = TRUE) check_for_config(cfg, ok_configs = c("ours", "fork")) - default_branch <- git_default_branch() + default_branch <- git_default_branch_(cfg) check_pr_branch(default_branch) challenge_uncommitted_changes() @@ -423,7 +423,7 @@ pr_push <- function() { pr_pull <- function() { cfg <- github_remote_config(github_get = TRUE) check_for_config(cfg) - default_branch <- git_default_branch() + default_branch <- git_default_branch_(cfg) check_pr_branch(default_branch) challenge_uncommitted_changes() @@ -449,11 +449,12 @@ pr_merge_main <- function() { #' @export #' @rdname pull-requests pr_view <- function(number = NULL, target = c("source", "primary")) { - tr <- target_repo(github_get = NA, role = target, ask = FALSE) + cfg <- github_remote_config(github_get = NA) + tr <- target_repo(cfg, github_get = NA, role = target, ask = FALSE) url <- NULL if (is.null(number)) { branch <- git_branch() - default_branch <- git_default_branch() + default_branch <- git_default_branch_(cfg) if (branch != default_branch) { url <- pr_url(branch = branch, tr = tr) if (is.null(url)) { @@ -491,11 +492,11 @@ pr_view <- function(number = NULL, target = c("source", "primary")) { #' @export #' @rdname pull-requests pr_pause <- function() { - # intentionally naive selection of target repo - tr <- target_repo(github_get = FALSE, ask = FALSE) + cfg <- github_remote_config(github_get = NA) + tr <- target_repo(cfg, github_get = NA, ask = FALSE) ui_bullets(c("v" = "Switching back to the default branch.")) - default_branch <- git_default_branch() + default_branch <- git_default_branch_(cfg) if (git_branch() == default_branch) { ui_bullets(c( "!" = "Already on this repo's default branch ({.val {default_branch}}), @@ -535,8 +536,10 @@ pr_clean <- function(number = NULL, withr::defer(rstudio_git_tickle()) mode <- match.arg(mode) repo <- git_repo() - tr <- target_repo(github_get = NA, role = target, ask = FALSE) - default_branch <- git_default_branch() + + cfg <- github_remote_config(github_get = NA) + tr <- target_repo(cfg, github_get = NA, role = target, ask = FALSE) + default_branch <- git_default_branch_(cfg) if (is.null(number)) { check_pr_branch(default_branch) @@ -629,14 +632,10 @@ pr_clean <- function(number = NULL, # we're in DEFAULT branch of a fork. I wish everyone set up DEFAULT to track the # DEFAULT branch in the source repo, but this protects us against sub-optimal # setup. -pr_pull_source_override <- function(tr = NULL, default_branch = NULL) { - # naive selection of target repo; calling function should analyse the config - tr <- tr %||% target_repo(github_get = FALSE, ask = FALSE) - +pr_pull_source_override <- function(tr, default_branch) { # TODO: why does this not use a check_*() function, i.e. shared helper? # I guess to issue a specific error message? current_branch <- git_branch() - default_branch <- default_branch %||% git_default_branch() if (current_branch != default_branch) { ui_abort(" Internal error: {.fun pr_pull_source_override} should only be used when on @@ -994,7 +993,7 @@ pr_branch_delete <- function(pr) { invisible(TRUE) } -check_pr_branch <- function(default_branch = git_default_branch()) { +check_pr_branch <- function(default_branch) { # the glue-ing happens inside check_current_branch(), where `gb` gives the # current git branch check_current_branch( diff --git a/tests/testthat/helper-mocks.R b/tests/testthat/helper-mocks.R index d2dc19ef4..d9e9d5757 100644 --- a/tests/testthat/helper-mocks.R +++ b/tests/testthat/helper-mocks.R @@ -28,7 +28,7 @@ local_ui_yep <- function(.env = caller_env()) { local_git_default_branch_remote <- function(.env = caller_env()) { local_mocked_bindings( - git_default_branch_remote = function(remote) { + git_default_branch_remote = function(cfg, remote) { list( name = remote, is_configured = TRUE, From b4e84773d1e5e12c60fa2bfc85b19819393fc517 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 2 Oct 2024 08:37:28 -0500 Subject: [PATCH 7/7] Specifically handle offline case in `pr_init()` (#2063) I realised that we're checking whether or not the host is online further down, so we might as well use that a little earlier to give a more informative message. --- R/pr.R | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/R/pr.R b/R/pr.R index 5d71233b8..f9134c5c5 100644 --- a/R/pr.R +++ b/R/pr.R @@ -167,30 +167,41 @@ pr_init <- function(branch) { cfg <- github_remote_config(github_get = NA) check_for_bad_config(cfg) tr <- target_repo(cfg, ask = FALSE) + online <- is_online(tr$host) - maybe_good_configs <- c("maybe_ours_or_theirs", "maybe_fork") - if (cfg$type %in% maybe_good_configs) { + if (!online) { ui_bullets(c( - "x" = 'Unable to confirm the GitHub remote configuration is - "pull request ready".', - "i" = "You probably need to configure a personal access token for - {.val {tr$host}}.", - "i" = "See {.run usethis::gh_token_help()} for help with that.", - "i" = "(Or maybe we're just offline?)" + "x" = "You are not currently online.", + "i" = "You can still create a local branch, but we can't check that your + current branch is up-to-date or setup the remote branch." )) - if (ui_github_remote_config_wat(cfg)) { + if (ui_nah("Do you want to continue?")) { ui_bullets(c("x" = "Cancelling.")) return(invisible()) } + } else { + maybe_good_configs <- c("maybe_ours_or_theirs", "maybe_fork") + if (cfg$type %in% maybe_good_configs) { + ui_bullets(c( + "x" = 'Unable to confirm the GitHub remote configuration is + "pull request ready".', + "i" = "You probably need to configure a personal access token for + {.val {tr$host}}.", + "i" = "See {.run usethis::gh_token_help()} for help with that.", + )) + if (ui_github_remote_config_wat(cfg)) { + ui_bullets(c("x" = "Cancelling.")) + return(invisible()) + } + } } - default_branch <- git_default_branch_(cfg) + default_branch <- if (online) git_default_branch_(cfg) else guess_local_default_branch() challenge_non_default_branch( "Are you sure you want to create a PR branch based on a non-default branch?", default_branch = default_branch ) - online <- is_online(tr$host) if (online) { # this is not pr_pull_source_override() because: # a) we may NOT be on default branch (although we probably are) @@ -213,10 +224,6 @@ pr_init <- function(branch) { ui_bullets(c("v" = "Pulling changes from {.val {remref}}.")) git_pull(remref = remref, verbose = FALSE) } - } else { - ui_bullets(c( - "!" = "Unable to pull changes for current branch, since we are offline." - )) } ui_bullets(c("v" = "Creating and switching to local branch {.val {branch}}."))