build_sitemap() improvements

* Only message if sitemap changed
* Simplify generation
* Escape file names. Fixes #2582.

NEWS.md

@@ -1,5 +1,6 @@
 # pkgdown (development version)
 
+* `build_sitemap()` now handles files with `<`/`>` in their name (#2582).
 * `build_reference_index()` now displays lifecycle badges next to the function name (#2123). 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).
 * New light switch makes it easy for users to switch between light and dark themes for the website (based on work in bslib by @gadenbuie). For now this behaviour is opt-in with `template.light-switch: true` but in the future we may turn it on automatically. See the customization vignette for details (#1696).

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

R/render.R

@@ -252,8 +252,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)
 }
 

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/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,10 @@ 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("build_sitemap() handles special xml characters", {
+  pkg <- local_pkgdown_site()
+  file_create(path(pkg$dst_path, "<.html"))
+
+  suppressMessages(build_sitemap(pkg))
+  expect_no_error(xml2::read_xml(path(pkg$dst_path, "sitemap.xml")))
 })