diff --git a/NEWS.md b/NEWS.md index 80117fa44..3ebef93b2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ * Preview links now work once again (#2435). * `build_home()` no longer renders Github issue and pull request templates (@hsloot, #2362) * It is now easier to preview parts of the website locally interactively. `build_reference_index()` and friends will call `init_site()` internally instead of erroring (@olivroy, #2329). +* Fixed an issue introduced in 2.0.8 where pkgdown was not using the Bootstrap version specified in a template package (@gadenbuie, #2443). # pkgdown 2.0.8 diff --git a/R/package.R b/R/package.R index 95a978143..fc0f49e43 100644 --- a/R/package.R +++ b/R/package.R @@ -24,12 +24,28 @@ as_pkgdown <- function(pkg = ".", override = list()) { meta <- read_meta(pkg) meta <- modify_list(meta, override) - bs_version <- get_bootstrap_version(list(meta = meta)) + # A local Bootstrap version, when provided, may drive the template choice + config_path <- pkgdown_config_path(pkg) + config_path <- if (!is.null(config_path)) fs::path_rel(config_path, pkg) + bs_version_local <- get_bootstrap_version( + template = meta$template, + config_path = config_path + ) template_config <- find_template_config( package = meta$template$package, - bs_version = bs_version + bs_version = bs_version_local ) + + bs_version_template <- + if (is.null(bs_version_local)) { + get_bootstrap_version( + template = template_config$template, + config_path = config_path, + package = meta$template$package + ) + } + meta <- modify_list(template_config, meta) # Ensure the URL has no trailing slash @@ -40,6 +56,12 @@ as_pkgdown <- function(pkg = ".", override = list()) { package <- desc$get_field("Package") version <- desc$get_field("Version") + # Check the final Bootstrap version, possibly filled in by template pkg + bs_version <- check_bootstrap_version( + bs_version_local %||% bs_version_template, + pkg + ) + development <- meta_development(meta, version, bs_version) if (is.null(meta$destination)) { @@ -98,26 +120,41 @@ read_desc <- function(path = ".") { desc::description$new(path) } -get_bootstrap_version <- function(pkg) { - template_bootstrap <- pkg$meta[["template"]]$bootstrap - template_bslib <- pkg$meta[["template"]]$bslib$version +get_bootstrap_version <- function(template, config_path = NULL, package = NULL) { + if (is.null(template)) { + return(NULL) + } + + template_bootstrap <- template[["bootstrap"]] + template_bslib <- template[["bslib"]][["version"]] if (!is.null(template_bootstrap) && !is.null(template_bslib)) { + instructions <- + if (!is.null(package)) { + paste0( + "Update the pkgdown config in {.pkg ", package, "}, ", + "or set a Bootstrap version in your {.file ", + if (is.null(config_path)) "_pkgdown.yml" else config_path, + "}." + ) + } else if (!is.null(config_path)) { + paste("Remove one of them from {.file", config_path, "}") + } + cli::cli_abort( c( sprintf( "Both {.field %s} and {.field %s} are set.", - pkgdown_field(pkg, c("template", "bootstrap")), - pkgdown_field(pkg, c("template", "bslib", "version")) + pkgdown_field(list(), c("template", "bootstrap")), + pkgdown_field(list(), c("template", "bslib", "version")) ), - x = "Remove one of them from {.file {pkgdown_config_relpath(pkg)}}" + i = instructions ), call = caller_env() ) } - version <- template_bootstrap %||% template_bslib - check_bootstrap_version(version, pkg) + template_bootstrap %||% template_bslib } check_bootstrap_version <- function(version, pkg) { diff --git a/R/pkgdown.R b/R/pkgdown.R index f5c41378e..53ada601b 100644 --- a/R/pkgdown.R +++ b/R/pkgdown.R @@ -52,3 +52,26 @@ local_pkgdown_site <- function(path = NULL, meta = NULL, env = parent.frame()) { pkg } + +local_pkgdown_template_pkg <- function(path = NULL, meta = NULL, env = parent.frame()) { + if (is.null(path)) { + path <- withr::local_tempdir(.local_envir = env) + desc <- desc::desc("!new") + desc$set("Package", "templatepackage") + desc$set("Title", "A test template package") + desc$write(file = file.path(path, "DESCRIPTION")) + } + + if (!is.null(meta)) { + path_pkgdown_yml <- fs::path(path, "inst", "pkgdown", "_pkgdown.yml") + fs::dir_create(fs::path_dir(path_pkgdown_yml)) + yaml::write_yaml(meta, path_pkgdown_yml) + } + + rlang::check_installed("pkgload") + pkgload::load_all(path) + withr::defer(pkgload::unload("templatepackage"), envir = env) + + path +} + diff --git a/tests/testthat/_snaps/templates.md b/tests/testthat/_snaps/templates.md new file mode 100644 index 000000000..0a744668b --- /dev/null +++ b/tests/testthat/_snaps/templates.md @@ -0,0 +1,18 @@ +# Invalid bootstrap version spec in template package + + Code + local_pkgdown_site(meta = list(template = list(package = "templatepackage"))) + Condition + Error in `as_pkgdown()`: + ! Both template.bootstrap and template.bslib.version are set. + i Update the pkgdown config in templatepackage, or set a Bootstrap version in your '_pkgdown.yml'. + +# Invalid bootstrap version spec in _pkgdown.yml + + Code + local_pkgdown_site(meta = list(template = list(bootstrap = 4, bslib = list( + version = 5)))) + Condition + Error in `as_pkgdown()`: + ! Both template.bootstrap and template.bslib.version are set. + diff --git a/tests/testthat/test-templates.R b/tests/testthat/test-templates.R index a871b4c92..a79d7aad0 100644 --- a/tests/testthat/test-templates.R +++ b/tests/testthat/test-templates.R @@ -92,3 +92,57 @@ test_that("BS5 templates have main + aside", { } }) + +# Bootstrap version resolution -------------------------------------------- +test_that("Bootstrap version in template package under `template.bootstrap`", { + path_template_package <- local_pkgdown_template_pkg( + meta = list(template = list(bootstrap = 5)) + ) + + pkg <- local_pkgdown_site(meta = list(template = list(package = "templatepackage"))) + + expect_equal(pkg$bs_version, 5) +}) + +test_that("Bootstrap version in template package under `template.bslib.version`", { + path_template_package <- local_pkgdown_template_pkg( + meta = list(template = list(bslib = list(version = 5))) + ) + + pkg <- local_pkgdown_site(meta = list(template = list(package = "templatepackage"))) + + expect_equal(pkg$bs_version, 5) +}) + +test_that("Invalid bootstrap version spec in template package", { + path_template_package <- local_pkgdown_template_pkg( + meta = list(template = list(bootstrap = 4, bslib = list(version = 5))) + ) + + expect_snapshot( + error = TRUE, + local_pkgdown_site(meta = list(template = list(package = "templatepackage"))) + ) +}) + +test_that("Invalid bootstrap version spec in _pkgdown.yml", { + expect_snapshot( + error = TRUE, + local_pkgdown_site(meta = list(template = list( + bootstrap = 4, bslib = list(version = 5) + ))) + ) +}) + +test_that("Valid local Bootstrap version masks invalid template package", { + path_template_package <- local_pkgdown_template_pkg( + meta = list(template = list(bootstrap = 4, bslib = list(version = 5))) + ) + + expect_no_error( + local_pkgdown_site(meta = list(template = list( + package = "templatepackage", + bootstrap = 5 + ))) + ) +})