Merge branch 'main' into math-rendering

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).

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)) {

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("<urlset xmlns = 'http://www.sitemaps.org/schemas/sitemap/0.9'></urlset>")
+  doc <- paste0(
+    "<urlset xmlns = 'http://www.sitemaps.org/schemas/sitemap/0.9'>\n",
+    paste0("<url><loc>", escape_html(urls), "</loc></url>\n", collapse = ""),
+    "</urlset>\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><loc>", url, "</loc></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)
 }

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}."
     ))
   }
 }

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"

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)
 }
 

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;

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
 

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 ------------------------------------------------------------
 

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

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())
 })
-

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())
 })

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`.