diff --git a/NEWS.md b/NEWS.md index 64051f04c..5a79b3a37 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ * New `template.math-rendering` allows you to control how math is rendered across your site. The default uses `mathml` which is low-dependency, but has the lowest fidelity. You can also use `mathjax`, the previous default, and `katex`, a faster alternative. (#1966). * Mathjax now uses version 3.2.2. +* `build_sitemap()` no longer includes redirected pages (#2582). * All external assets (JS, CSS, fonts) are now directly included in the site instead of fetched from external CDN (@salim-b, #2249) * `build_reference_index()` now displays function lifecycle badges next to the function name (#2123). The badges are extracted only from the function description. You can now also use `has_lifecycle()` to select functions by their lifecycle status. * `build_articles()` now recognises a new `external-articles` top-level field that allows you to define articles that live in other packages (#2028). diff --git a/R/build-redirects.R b/R/build-redirects.R index d61b7cf72..f4e4fc735 100644 --- a/R/build-redirects.R +++ b/R/build-redirects.R @@ -82,12 +82,19 @@ reference_redirects <- function(pkg) { names(redirects) <- paste0(names(redirects), ".html") + # Ensure we don't create an invalid file name + redirects <- redirects[valid_filename(names(redirects))] + # Ensure we don't override an existing file redirects <- redirects[setdiff(names(redirects), pkg$topics$file_out)] unname(purrr::imap(redirects, function(to, from) paste0("reference/", c(from, to)))) } +valid_filename <- function(x) { + x == path_sanitize(x) +} + article_redirects <- function(pkg) { is_vig_in_articles <- path_has_parent(pkg$vignettes$name, "articles") if (!any(is_vig_in_articles)) { diff --git a/R/build-search-docs.R b/R/build-search-docs.R index 6ce4ded87..53409a6b1 100644 --- a/R/build-search-docs.R +++ b/R/build-search-docs.R @@ -35,35 +35,16 @@ build_sitemap <- function(pkg = ".") { } urls <- paste0(url, get_site_paths(pkg)) - - doc <- xml2::read_xml( - paste0("") + doc <- paste0( + "\n", + paste0("", escape_html(urls), "\n", collapse = ""), + "\n" ) - url_nodes <- unwrap_purrr_error(purrr::map(urls, url_node)) - for (url in url_nodes) { - xml2::xml_add_child(doc, url) - } - - xml_path <- path(pkg$dst_path, "sitemap.xml") - cli::cli_inform("Writing {dst_path(path_rel(xml_path, pkg$dst_path))}") - - xml2::write_xml(doc, file = xml_path) - + write_if_different(pkg, doc, "sitemap.xml", check = FALSE) invisible() } -url_node <- function(url) { - withCallingHandlers( - xml2::read_xml( - paste0("", url, "") - ), - error = function(err) { - cli::cli_abort("Failed to wrap URL {.str {url}}", parent = err) - } - ) -} - #' Build search index #' #' @description @@ -315,5 +296,9 @@ get_site_paths <- function(pkg) { # do not include dev package website in search index / sitemap dev_destination <- meta_development(pkg$meta, pkg$version)$destination - paths_rel[!path_has_parent(paths_rel, "dev")] + paths_rel <- paths_rel[!path_has_parent(paths_rel, "dev")] + + # do not include redirects + redirects <- purrr::map_chr(data_redirects(pkg, has_url = TRUE), 1) + setdiff(paths_rel, redirects) } diff --git a/R/check-built.R b/R/check-built.R index dc6e501bf..3f78e2a30 100644 --- a/R/check-built.R +++ b/R/check-built.R @@ -33,7 +33,7 @@ check_missing_images <- function(pkg, src_path, dst_path) { cli::cli_inform(c( x = "Missing alt-text in {.file {path_rel(src_path, pkg$src_path)}}", set_names(problems, "*"), - i = "Learn more in {.vignette accessibility}." + i = "Learn more in {.vignette pkgdown::accessibility}." )) } } diff --git a/R/navbar-menu.R b/R/navbar-menu.R index f9c007952..c981f41f1 100644 --- a/R/navbar-menu.R +++ b/R/navbar-menu.R @@ -226,7 +226,7 @@ navbar_html_text <- function(x) { c( x = "Icon {.str {x$icon}} lacks an {.var aria-label}.", i = "Specify {.var aria-label} to make the icon accessible to screen readers.", - i = "Learn more in {.vignette accessibility}." + i = "Learn more in {.vignette pkgdown::accessibility}." ), .frequency = "regularly", .frequency_id = "icon-aria-label" diff --git a/R/render.R b/R/render.R index f992bc6ae..96e05e541 100644 --- a/R/render.R +++ b/R/render.R @@ -254,8 +254,8 @@ same_contents <- function(path, contents) { new_hash <- digest::digest(contents, serialize = FALSE) cur_contents <- paste0(read_lines(path), collapse = "\n") - cur_hash <- digest::digest(cur_contents, serialize = FALSE) - + cur_hash <- digest::digest(cur_contents, serialize = FALSE) + identical(new_hash, cur_hash) } diff --git a/inst/BS5/assets/pkgdown.scss b/inst/BS5/assets/pkgdown.scss index 911382fe7..8e29c78fa 100644 --- a/inst/BS5/assets/pkgdown.scss +++ b/inst/BS5/assets/pkgdown.scss @@ -122,7 +122,7 @@ aside { } // Add some visual distinction between content and "sidebar" on mobile -@include media-breakpoint-down(sm) { +@include media-breakpoint-down(md) { // Additional specificity needed to override bootstrap width on .row > * .row > aside { margin: 0.5rem; diff --git a/tests/testthat/_snaps/build-article.md b/tests/testthat/_snaps/build-article.md index 0535c3f1d..aa0fb6de8 100644 --- a/tests/testthat/_snaps/build-article.md +++ b/tests/testthat/_snaps/build-article.md @@ -20,7 +20,7 @@ x Missing alt-text in 'vignettes/missing-images.Rmd' * kitten.jpg * missing-images_files/figure-html/unnamed-chunk-1-1.png - i Learn more in `vignette(accessibility)`. + i Learn more in `vignette(pkgdown::accessibility)`. # can build article that uses html_vignette diff --git a/tests/testthat/_snaps/build-search-docs.md b/tests/testthat/_snaps/build-search-docs.md index 8b97b6be3..02b4825a4 100644 --- a/tests/testthat/_snaps/build-search-docs.md +++ b/tests/testthat/_snaps/build-search-docs.md @@ -1,10 +1,12 @@ -# url_node gives informative error +# build sitemap only messages when it updates Code - url_node("<") - Condition - Error in `url_node()`: - ! Failed to wrap URL "<" - Caused by error in `read_xml.raw()`: - ! StartTag: invalid element name [68] + build_sitemap(pkg) + Message + -- Building sitemap ------------------------------------------------------------ + Writing `sitemap.xml` + Code + build_sitemap(pkg) + Message + -- Building sitemap ------------------------------------------------------------ diff --git a/tests/testthat/_snaps/navbar-menu.md b/tests/testthat/_snaps/navbar-menu.md index 3a70f64e3..257261f5f 100644 --- a/tests/testthat/_snaps/navbar-menu.md +++ b/tests/testthat/_snaps/navbar-menu.md @@ -52,7 +52,7 @@ Message x Icon "fa-question" lacks an `aria-label`. i Specify `aria-label` to make the icon accessible to screen readers. - i Learn more in `vignette(accessibility)`. + i Learn more in `vignette(pkgdown::accessibility)`. This message is displayed once every 8 hours. # can construct theme menu diff --git a/tests/testthat/test-build-redirects.R b/tests/testthat/test-build-redirects.R index 5b254cd70..ddc8bc87d 100644 --- a/tests/testthat/test-build-redirects.R +++ b/tests/testthat/test-build-redirects.R @@ -63,6 +63,21 @@ test_that("generates redirects only for non-name aliases", { ) }) +test_that("doesn't generates redirect for aliases that can't be file names", { + pkg <- list( + meta = list(url = "http://foo.com"), + topics = list( + name = "bar", + alias = list(c("bar", "baz", "[<-.baz")), + file_out = "bar.html" + ) + ) + expect_equal( + reference_redirects(pkg), + list(c("reference/baz.html", "reference/bar.html")) + ) +}) + test_that("never redirects away from existing topic", { pkg <- list( meta = list(url = "http://foo.com"), @@ -89,4 +104,3 @@ test_that("no redirects if no aliases", { ) expect_equal(reference_redirects(pkg), list()) }) - diff --git a/tests/testthat/test-build-search-docs.R b/tests/testthat/test-build-search-docs.R index 6bec99bbe..d9432c482 100644 --- a/tests/testthat/test-build-search-docs.R +++ b/tests/testthat/test-build-search-docs.R @@ -31,6 +31,21 @@ test_that("build_search() builds the expected search.json with an URL", { expect_snapshot_file(json_path, "search.json") }) +test_that("build sitemap only messages when it updates", { + pkg <- local_pkgdown_site(test_path("assets/news"), ' + url: https://example.com + template: + bootstrap: 5 + ') + + suppressMessages(init_site(pkg)) + suppressMessages(build_home(pkg)) + expect_snapshot({ + build_sitemap(pkg) + build_sitemap(pkg) + }) +}) + test_that("build_search() builds the expected search.json with no URL", { pkg <- local_pkgdown_site(test_path("assets/news"), ' template: @@ -51,6 +66,11 @@ test_that("build_search() builds the expected search.json with no URL", { expect_snapshot_file(json_path, "search-no-url.json") }) -test_that("url_node gives informative error", { - expect_snapshot(url_node("<"), error = TRUE) +test_that("sitemap excludes redirects", { + pkg <- local_pkgdown_site(meta = list( + url = "https://example.com", + redirects = list(c("a.html", "b.html")) + )) + suppressMessages(build_redirects(pkg)) + expect_equal(get_site_paths(pkg), character()) }) diff --git a/vignettes/customise.Rmd b/vignettes/customise.Rmd index 243835318..5eae8c1d5 100644 --- a/vignettes/customise.Rmd +++ b/vignettes/customise.Rmd @@ -45,7 +45,7 @@ The following sections show you how. Please note that pkgdown's default theme has been carefully optimised to be accessible, so if you make changes, make sure that to also read `vignette("accessibility")` to learn about potential accessibility pitfalls. -### Light switch +### Light switch {#light-switch} You can provide a "light switch" to allow your uses to switch between dark and light themes by setting the `light-switch` template option to true: @@ -266,10 +266,10 @@ This is the default structure: navbar: structure: left: [intro, reference, articles, tutorials, news] - right: [search, github] + right: [search, github, lightswitch] ``` -It makes use of the the six built-in components: +It makes use of the the following built-in components: - `intro`: "Get Started", which links to a vignette or article with the same name as the package[^dots]. - `reference`, if there are any `.Rd` files. @@ -278,6 +278,7 @@ It makes use of the the six built-in components: - `news`, if `NEWS.md` exists. - `search`, the search box (see `?build_search` for more details). - `github`, a link to the source repository (with an icon), if it can be automatically determined from the `DESCRIPTION`. +- `lightswitch`, a ["light switch"](#light-switch) to select light mode, dark mode, or auto mode. [^dots]: Note that dots (`.`) in the package name need to be replaced by hyphens (`-`) in the vignette filename to be recognized as the intro. That means for a package `foo.bar` the intro needs to be named `foo-bar.Rmd`.