From f82c6dc4bd66370ca0f41cf823e34d2cb1c4d356 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Thu, 30 May 2024 17:42:24 -0500 Subject: [PATCH 1/4] Check that favicons are present and up-to-date --- NEWS.md | 1 + R/build-favicons.R | 5 ++++- R/check.R | 26 +++++++++++++++++++++++++- R/pkgdown.R | 15 ++++++++++----- R/render.R | 1 - logo.svg | 0 tests/testthat/_snaps/check.md | 23 +++++++++++++++++++++++ tests/testthat/test-check.R | 27 ++++++++++++++++++++++++++- 8 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 logo.svg diff --git a/NEWS.md b/NEWS.md index 7fad3a2c7..2531d3f18 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # pkgdown (development version) +* `pkgdown_sitrep()`/`check_pkgdown()` now check that you have up-to-date favicons if you have a package logo. * pkgdown now uses httr2 instead of httr (#2600). * 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. diff --git a/R/build-favicons.R b/R/build-favicons.R index 8c0070c8e..252f7fa61 100644 --- a/R/build-favicons.R +++ b/R/build-favicons.R @@ -112,6 +112,9 @@ copy_favicons <- function(pkg = ".") { has_favicons <- function(pkg = ".") { pkg <- as_pkgdown(pkg) + unname(file_exists(path_favicons(pkg))) +} - unname(file_exists(path(pkg$src_path, "pkgdown", "favicon"))) +path_favicons <- function(pkg) { + path(pkg$src_path, "pkgdown", "favicon") } diff --git a/R/check.R b/R/check.R index 881566fc0..324346ae3 100644 --- a/R/check.R +++ b/R/check.R @@ -43,9 +43,11 @@ pkgdown_sitrep <- function(pkg = ".") { } error_to_sitrep("URLs", check_urls(pkg)) + error_to_sitrep("Favicons", check_favicons(pkg)) error_to_sitrep("Open graph metadata", data_open_graph(pkg)) error_to_sitrep("Articles metadata", data_articles_index(pkg)) error_to_sitrep("Reference metadata", data_reference_index(pkg)) + } error_to_sitrep <- function(title, code) { @@ -83,7 +85,6 @@ check_urls <- function(pkg = ".", call = caller_env()) { } else { desc_urls <- pkg$desc$get_urls() desc_urls <- sub("/$", "", desc_urls) - if (!pkg$meta[["url"]] %in% desc_urls) { cli::cli_abort( c("{.file DESCRIPTION} {.field URL} lacks package url ({url}).", details), @@ -92,3 +93,26 @@ check_urls <- function(pkg = ".", call = caller_env()) { } } } + +check_favicons <- function(pkg) { + if (!has_logo(pkg)) { + return() + } + + if (has_favicons(pkg)) { + logo <- find_logo(pkg$src_path) + favicon <- path(path_favicons(pkg), "favicon.ico") + + if (out_of_date(logo, favicon)) { + cli::cli_abort(c( + "Package logo is newer than favicons.", + i = "Do you need to rerun {.fn build_favicons}?" + )) + } + } else { + cli::cli_abort(c( + "Found package logo but not favicons.", + i = "Do you need to run {.fn build_favicons}?" + )) + } +} diff --git a/R/pkgdown.R b/R/pkgdown.R index 61e98db9b..3ecf33449 100644 --- a/R/pkgdown.R +++ b/R/pkgdown.R @@ -18,7 +18,10 @@ local_envvar_pkgdown <- function(pkg, scope = parent.frame()) { ) } -local_pkgdown_site <- function(path = NULL, meta = list(), env = caller_env()) { +local_pkgdown_site <- function(path = NULL, + meta = list(), + desc = list(), + env = caller_env()) { check_string(path, allow_null = TRUE) dst_path <- withr::local_tempdir(.local_envir = env) @@ -27,10 +30,12 @@ local_pkgdown_site <- function(path = NULL, meta = list(), env = caller_env()) { if (is.null(path)) { path <- withr::local_tempdir(.local_envir = env) - desc <- desc::desc("!new") - desc$set("Package", "testpackage") - desc$set("Title", "A test package") - desc$write(file = path(path, "DESCRIPTION")) + description <- desc::desc("!new") + description$set("Package", "testpackage") + description$set("Title", "A test package") + if (length(desc) > 0) + inject(description$set(!!!desc)) + description$write(file = path(path, "DESCRIPTION")) # Default to BS5 only if template not specified meta$template <- meta$template %||% list(bootstrap = 5) diff --git a/R/render.R b/R/render.R index 7c52654f7..50363200b 100644 --- a/R/render.R +++ b/R/render.R @@ -273,7 +273,6 @@ file_digest <- function(path) { } } - made_by_pkgdown <- function(path) { if (!file_exists(path)) return(TRUE) diff --git a/logo.svg b/logo.svg new file mode 100644 index 000000000..e69de29bb diff --git a/tests/testthat/_snaps/check.md b/tests/testthat/_snaps/check.md index 52d533696..35aced655 100644 --- a/tests/testthat/_snaps/check.md +++ b/tests/testthat/_snaps/check.md @@ -7,6 +7,9 @@ x Bootstrap 3 is deprecated; please switch to Bootstrap 5. i Learn more at . v URLs ok. + x Favicons not ok. + Found package logo but not favicons. + Do you need to run `build_favicons()`? v Open graph metadata ok. v Articles metadata ok. v Reference metadata ok. @@ -20,6 +23,7 @@ x URLs not ok. 'DESCRIPTION' URL lacks package url (http://test.org). See details in `vignette(pkgdown::metadata)`. + v Favicons ok. v Open graph metadata ok. v Articles metadata ok. x Reference metadata not ok. @@ -43,6 +47,7 @@ Message -- Sitrep ---------------------------------------------------------------------- v URLs ok. + v Favicons ok. v Open graph metadata ok. v Articles metadata ok. v Reference metadata ok. @@ -69,3 +74,21 @@ ! 'DESCRIPTION' URL lacks package url (https://testpackage.r-lib.org). i See details in `vignette(pkgdown::metadata)`. +# check_favicons reports problems + + Code + check_favicons(pkg) + Condition + Error in `check_favicons()`: + ! Found package logo but not favicons. + i Do you need to run `build_favicons()`? + +--- + + Code + check_favicons(pkg) + Condition + Error in `check_favicons()`: + ! Package logo is newer than favicons. + i Do you need to rerun `build_favicons()`? + diff --git a/tests/testthat/test-check.R b/tests/testthat/test-check.R index 2653c70b5..2ecbf023d 100644 --- a/tests/testthat/test-check.R +++ b/tests/testthat/test-check.R @@ -29,7 +29,11 @@ test_that("checks fails on first problem", { }) test_that("both inform if everything is ok", { - pkg <- test_path("assets/open-graph") + pkg <- local_pkgdown_site( + meta = list(url = "https://example.com"), + desc = list(URL = "https://example.com") + ) + expect_snapshot({ pkgdown_sitrep(pkg) check_pkgdown(pkg) @@ -47,3 +51,24 @@ test_that("check_urls reports problems", { pkg <- test_path("assets/cname") expect_snapshot(check_urls(pkg), error = TRUE) }) + +# check favicons -------------------------------------------------------------- + +test_that("check_favicons reports problems", { + pkg <- local_pkgdown_site() + # no logo no problems + expect_no_error(check_favicons(pkg)) + + # logo but no favicons + file_touch(path(pkg$src_path, "logo.svg")) + expect_snapshot(check_favicons(pkg), error = TRUE) + + # logo and old favicons + dir_create(path_favicons(pkg)) + file_touch(path(path_favicons(pkg), "favicon.ico"), Sys.time() - 86400) + expect_snapshot(check_favicons(pkg), error = TRUE) + + # logo and new favicons + file_touch(path(path_favicons(pkg), "favicon.ico"), Sys.time() + 86400) + expect_no_error(check_favicons(pkg)) +}) From e6df1a4f4efc2b5b37a6eca7d51b9ad8e4253712 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 31 May 2024 07:28:32 -0500 Subject: [PATCH 2/4] Update tests/testthat/test-check.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Maëlle Salmon --- tests/testthat/test-check.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-check.R b/tests/testthat/test-check.R index 2ecbf023d..4038972f4 100644 --- a/tests/testthat/test-check.R +++ b/tests/testthat/test-check.R @@ -56,6 +56,7 @@ test_that("check_urls reports problems", { test_that("check_favicons reports problems", { pkg <- local_pkgdown_site() + # no logo no problems expect_no_error(check_favicons(pkg)) From 10b88cd6294f42f9578536b16f14944fd46a4dcc Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 31 May 2024 07:29:05 -0500 Subject: [PATCH 3/4] Remove accidentally committed file --- logo.svg | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 logo.svg diff --git a/logo.svg b/logo.svg deleted file mode 100644 index e69de29bb..000000000 From 87b8fdb127d3db6b63356080d470fb63784051e7 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 31 May 2024 08:26:33 -0500 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: olivroy <52606734+olivroy@users.noreply.github.com> --- R/check.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/check.R b/R/check.R index 324346ae3..93e660caa 100644 --- a/R/check.R +++ b/R/check.R @@ -106,13 +106,13 @@ check_favicons <- function(pkg) { if (out_of_date(logo, favicon)) { cli::cli_abort(c( "Package logo is newer than favicons.", - i = "Do you need to rerun {.fn build_favicons}?" + i = "Do you need to rerun {.run [build_favicons()](pkgdown::build_favicons())}?" )) } } else { cli::cli_abort(c( "Found package logo but not favicons.", - i = "Do you need to run {.fn build_favicons}?" + i = "Do you need to run {.run [build_favicons()](pkgdown::build_favicons())}?" )) } }