diff --git a/NEWS.md b/NEWS.md index 0e7c1e20d6..88363efb5c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # pkgdown (development version) +* `check_pkgdown()` and `pkgdown_sitrep()` have been unified so that they both report on the same problems. They now only differ in the style of their output: `pkgdown_sitrep()` reports whether each category is ok or not ok, while `check_pkgdown()` errors on the first issue (#2463). +* `build_site()` automatically runs `pkgdown_sitrep()` at the start of the process (#2380). * New `vignette("accessibility")` describes what manual tasks you need to perform to make your site as accessible as possible (#2344). * `build_reference()` now automatically translates `--`, `---`, ``` `` ```, and `''` to their unicode equivalents (#2530). * Tweaked navbar display on mobile so that long titles in drop downs (e.g. article titles) are now wrapped, and the search input spans the full width (#2512). diff --git a/R/build-articles.R b/R/build-articles.R index ce70a11a4f..1b56b752e2 100644 --- a/R/build-articles.R +++ b/R/build-articles.R @@ -379,9 +379,9 @@ data_articles_index <- function(pkg = ".") { pkg <- as_pkgdown(pkg) meta <- pkg$meta$articles %||% default_articles_index(pkg) - sections <- meta %>% - purrr::map(data_articles_index_section, pkg = pkg) %>% - purrr::compact() + sections <- unwrap_purrr_error(meta %>% + purrr::imap(data_articles_index_section, pkg = pkg) %>% + purrr::compact()) # Check for unlisted vignettes listed <- sections %>% @@ -411,13 +411,9 @@ data_articles_index <- function(pkg = ".") { )) } -data_articles_index_section <- function(section, pkg) { - if (!set_contains(names(section), c("title", "contents"))) { - cli::cli_abort( - "Section must have components {.field title}, {.field contents}", - call = caller_env() - ) - } +data_articles_index_section <- function(section, index, pkg) { + id <- section$title %||% section$subtitle %||% index + check_contents(section$contents, id, pkg, quote(build_articles())) # Match topics against any aliases in_section <- select_vignettes(section$contents, pkg$vignettes) diff --git a/R/build-reference-index.R b/R/build-reference-index.R index cbaba6d5f8..3e8be4c157 100644 --- a/R/build-reference-index.R +++ b/R/build-reference-index.R @@ -57,7 +57,7 @@ data_reference_index_rows <- function(section, index, pkg) { if (has_name(section, "contents")) { id <- section$title %||% section$subtitle %||% index - check_contents(section$contents, id, pkg) + check_contents(section$contents, id, pkg, quote(build_reference_index())) topics <- section_topics(section$contents, pkg$topics, pkg$src_path) names <- topics$name @@ -74,9 +74,7 @@ data_reference_index_rows <- function(section, index, pkg) { purrr::compact(rows) } -check_contents <- function(contents, id, pkg) { - call <- quote(build_reference_index()) - +check_contents <- function(contents, id, pkg, call = caller_env()) { if (length(contents) == 0) { config_abort( pkg, diff --git a/R/build.R b/R/build.R index 84a061c5b5..2c5f5e9cfd 100644 --- a/R/build.R +++ b/R/build.R @@ -443,6 +443,8 @@ build_site_local <- function(pkg = ".", cli::cli_inform("Reading from: {src_path(path_abs(pkg$src_path))}") cli::cli_inform("Writing to: {dst_path(path_abs(pkg$dst_path))}") + pkgdown_sitrep(pkg) + init_site(pkg) build_home(pkg, override = override, preview = FALSE) diff --git a/R/check-built.R b/R/check-built.R new file mode 100644 index 0000000000..e0ae664403 --- /dev/null +++ b/R/check-built.R @@ -0,0 +1,27 @@ + +check_built_site <- function(pkg = ".") { + pkg <- as_pkgdown(pkg) + + cli::cli_rule("Checking for problems") + index_path <- path_index(pkg) + if (!is.null(index_path)) { + check_missing_images(pkg, index_path, "index.html") + } +} + +check_missing_images <- function(pkg, src_path, dst_path) { + html <- xml2::read_html(path(pkg$dst_path, dst_path), encoding = "UTF-8") + src <- xml2::xml_attr(xml2::xml_find_all(html, ".//img"), "src") + + rel_src <- src[xml2::url_parse(src)$scheme == ""] + rel_path <- fs::path_norm(path(fs::path_dir(dst_path), rel_src)) + exists <- fs::file_exists(path(pkg$dst_path, rel_path)) + + if (any(!exists)) { + paths <- rel_src[!exists] + cli::cli_warn(c( + "Missing images in {.file {path_rel(src_path, pkg$src_path)}}: {.file {paths}}", + i = "pkgdown can only use images in {.file man/figures} and {.file vignettes}" + )) + } +} diff --git a/R/check.R b/R/check.R index 4a5b57bfad..ac4ceab031 100644 --- a/R/check.R +++ b/R/check.R @@ -1,20 +1,27 @@ #' Check `_pkgdown.yml` #' #' @description -#' Check that your `_pkgdown.yml` is valid without building the whole -#' site. Currently this: +#' This pair of functions checks that your `_pkgdown.yml` is valid without +#' building the whole site. `check_pkgdown()` errors at the first problem; +#' `pkgdown_sitrep()` reports the status of all checks. +#' +#' Currently they check that: #' -#' * Checks the reference and article indexes to ensure that pkgdown can -#' read them, and that every documentation topic and vignette/article is -#' included in the index. +#' * There's a `url` in the pkgdown configuration, which is also recorded +#' in the `URL` field of the `DESCRIPTION`. #' -#' * Validates any opengraph metadata that you might have supplied +#' * All opengraph metadata is valid. #' +#' * All reference topics are included in the index. +#' +#' * All articles/vignettes are included in the index. +# #' @export #' @inheritParams as_pkgdown check_pkgdown <- function(pkg = ".") { pkg <- as_pkgdown(pkg) + check_urls(pkg) data_open_graph(pkg) data_articles_index(pkg) data_reference_index(pkg) @@ -22,29 +29,59 @@ check_pkgdown <- function(pkg = ".") { cli::cli_inform(c("v" = "No problems found.")) } -check_built_site <- function(pkg = ".") { +#' @export +#' @rdname check_pkgdown +pkgdown_sitrep <- function(pkg = ".") { + cli::cli_rule("Sitrep") + pkg <- as_pkgdown(pkg) + error_to_sitrep("URLs", check_urls(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)) +} - cli::cli_rule("Checking for problems") - index_path <- path_index(pkg) - if (!is.null(index_path)) { - check_missing_images(pkg, index_path, "index.html") - } +error_to_sitrep <- function(title, code) { + tryCatch( + { + code + cli::cli_inform(c("v" = "{title} ok.")) + }, + rlang_error = function(e) { + bullets <- c(cnd_header(e), cnd_body(e)) + cli::cli_inform(c(x = "{title} not ok.", set_names(bullets, " "))) + } + ) + invisible() } -check_missing_images <- function(pkg, src_path, dst_path) { - html <- xml2::read_html(path(pkg$dst_path, dst_path), encoding = "UTF-8") - src <- xml2::xml_attr(xml2::xml_find_all(html, ".//img"), "src") +check_urls <- function(pkg = ".", call = caller_env()) { + pkg <- as_pkgdown(pkg) + details <- c(i = "See details in {.vignette pkgdown::metadata}.") + + if (identical(pkg$meta, list())) { + cli::cli_abort( + c("No {.path _pkgdown.yml} found.", details), + call = call + ) + } - rel_src <- src[xml2::url_parse(src)$scheme == ""] - rel_path <- fs::path_norm(path(fs::path_dir(dst_path), rel_src)) - exists <- fs::file_exists(path(pkg$dst_path, rel_path)) + url <- pkg$meta[["url"]] - if (any(!exists)) { - paths <- rel_src[!exists] - cli::cli_warn(c( - "Missing images in {.file {path_rel(src_path, pkg$src_path)}}: {.file {paths}}", - i = "pkgdown can only use images in {.file man/figures} and {.file vignettes}" - )) + if (is.null(url)) { + cli::cli_abort( + c("{config_path(pkg)} lacks {.field url}.", details), + call = call + ) + } 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), + call = call + ) + } } } diff --git a/R/sitrep.R b/R/sitrep.R deleted file mode 100644 index 74505da9fc..0000000000 --- a/R/sitrep.R +++ /dev/null @@ -1,47 +0,0 @@ -#' Report package pkgdown situation -#' -#' @description -#' -#' `pkgdown_sitrep()` reports -#' -#' * If there is an `url` field in the pkgdown configuration; -#' -#' * If that pkgdown website URL is stored in the DESCRIPTION file. -#' -#' @inheritParams as_pkgdown -#' -#' @export -#' -pkgdown_sitrep <- function(pkg = ".") { - pkg <- as_pkgdown(pkg) - warns <- c() - - if (is.null(pkg$meta[["url"]])) { - warns <- c( - warns, - x = "{.field url} is absent. See {.vignette pkgdown::metadata}." - ) - } - - desc_urls <- pkg$desc$get_urls() - desc_urls <- sub("/$", "", desc_urls) - if (length(desc_urls) == 0 || !pkg$meta[["url"]] %in% desc_urls) { - warns <- c(warns, x = "{.file DESCRIPTION} {.field URL} is empty.") - } - - if (length(warns) == 0) { - cli::cli_inform(c( - "v" = "pkgdown situation report: {.emph {cli::col_green('all clear')}}", - "!" = "{.emph Double-check the following URLs:}", - " " = "{config_path(pkg)} contains URL {.url {pkg$meta['url']}}", - " " = "{.file DESCRIPTION} contains URL{?s} {.url {desc_urls}}" - )) - } else { - cli::cli_warn(c( - "pkgdown situation report: {.emph {cli::col_red('configuration error')}}", - warns - )) - } - - invisible() -} diff --git a/man/check_pkgdown.Rd b/man/check_pkgdown.Rd index d049c9439a..3854e8ca6a 100644 --- a/man/check_pkgdown.Rd +++ b/man/check_pkgdown.Rd @@ -2,20 +2,27 @@ % Please edit documentation in R/check.R \name{check_pkgdown} \alias{check_pkgdown} +\alias{pkgdown_sitrep} \title{Check \verb{_pkgdown.yml}} \usage{ check_pkgdown(pkg = ".") + +pkgdown_sitrep(pkg = ".") } \arguments{ \item{pkg}{Path to package.} } \description{ -Check that your \verb{_pkgdown.yml} is valid without building the whole -site. Currently this: +This pair of functions checks that your \verb{_pkgdown.yml} is valid without +building the whole site. \code{check_pkgdown()} errors at the first problem; +\code{pkgdown_sitrep()} reports the status of all checks. + +Currently they check that: \itemize{ -\item Checks the reference and article indexes to ensure that pkgdown can -read them, and that every documentation topic and vignette/article is -included in the index. -\item Validates any opengraph metadata that you might have supplied +\item There's a \code{url} in the pkgdown configuration, which is also recorded +in the \code{URL} field of the \code{DESCRIPTION}. +\item All opengraph metadata is valid. +\item All reference topics are included in the index. +\item All articles/vignettes are included in the index. } } diff --git a/man/pkgdown_sitrep.Rd b/man/pkgdown_sitrep.Rd deleted file mode 100644 index 90ada03344..0000000000 --- a/man/pkgdown_sitrep.Rd +++ /dev/null @@ -1,18 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/sitrep.R -\name{pkgdown_sitrep} -\alias{pkgdown_sitrep} -\title{Report package pkgdown situation} -\usage{ -pkgdown_sitrep(pkg = ".") -} -\arguments{ -\item{pkg}{Path to package.} -} -\description{ -\code{pkgdown_sitrep()} reports -\itemize{ -\item If there is an \code{url} field in the pkgdown configuration; -\item If that pkgdown website URL is stored in the DESCRIPTION file. -} -} diff --git a/tests/testthat/_snaps/check-built.md b/tests/testthat/_snaps/check-built.md new file mode 100644 index 0000000000..0f750c9208 --- /dev/null +++ b/tests/testthat/_snaps/check-built.md @@ -0,0 +1,11 @@ +# warn about missing images in readme + + Code + check_built_site(pkg) + Message + -- Checking for problems ------------------------------------------------------- + Condition + Warning: + Missing images in 'README.md': 'articles/kitten.jpg' + i pkgdown can only use images in 'man/figures' and 'vignettes' + diff --git a/tests/testthat/_snaps/check.md b/tests/testthat/_snaps/check.md index a5479cafc0..08744c86db 100644 --- a/tests/testthat/_snaps/check.md +++ b/tests/testthat/_snaps/check.md @@ -1,37 +1,58 @@ -# fails if reference index incomplete +# sitrep reports all problems Code - check_pkgdown(pkg) - Condition - Error in `check_pkgdown()`: - ! 1 topic missing from index: "?". - i Either use `@keywords internal` to drop from index, or - i Edit _pkgdown.yml to fix the problem. - -# fails if article index incomplete + pkgdown_sitrep(pkg) + Message + -- Sitrep ---------------------------------------------------------------------- + x URLs not ok. + 'DESCRIPTION' URL lacks package url (http://test.org). + See details in `vignette(pkgdown::metadata)`. + v Open graph metadata ok. + v Articles metadata ok. + x Reference metadata not ok. + 1 topic missing from index: "?". + Either use `@keywords internal` to drop from index, or + Edit _pkgdown.yml to fix the problem. + +# checks fails on first problem Code check_pkgdown(pkg) Condition Error in `check_pkgdown()`: - ! 2 vignettes missing from index: "articles/nested" and "width". - i Edit _pkgdown.yml to fix the problem. + ! 'DESCRIPTION' URL lacks package url (http://test.org). + i See details in `vignette(pkgdown::metadata)`. -# informs if everything is ok +# both inform if everything is ok + Code + pkgdown_sitrep(pkg) + Message + -- Sitrep ---------------------------------------------------------------------- + v URLs ok. + v Open graph metadata ok. + v Articles metadata ok. + v Reference metadata ok. Code check_pkgdown(pkg) Message v No problems found. -# warn about missing images in readme +# check_urls reports problems Code - check_built_site(pkg) - Message - -- Checking for problems ------------------------------------------------------- + check_urls(pkg) + Condition + Error: + ! _pkgdown.yml lacks url. + i See details in `vignette(pkgdown::metadata)`. + +--- + + Code + check_urls(pkg) Condition - Warning: - Missing images in 'README.md': 'articles/kitten.jpg' - i pkgdown can only use images in 'man/figures' and 'vignettes' + Error: + ! 'DESCRIPTION' URL lacks package url (https://testpackage.r-lib.org). + i See details in `vignette(pkgdown::metadata)`. diff --git a/tests/testthat/_snaps/sitrep.md b/tests/testthat/_snaps/sitrep.md deleted file mode 100644 index 60db031677..0000000000 --- a/tests/testthat/_snaps/sitrep.md +++ /dev/null @@ -1,29 +0,0 @@ -# pkgdown_sitrep works - - Code - pkgdown_sitrep(pkg) - Condition - Warning: - pkgdown situation report: configuration error - x url is absent. See `vignette(pkgdown::metadata)`. - x 'DESCRIPTION' URL is empty. - ---- - - Code - pkgdown_sitrep(pkg) - Condition - Warning: - pkgdown situation report: configuration error - x 'DESCRIPTION' URL is empty. - ---- - - Code - pkgdown_sitrep(pkg) - Message - v pkgdown situation report: all clear - ! Double-check the following URLs: - _pkgdown.yml contains URL - 'DESCRIPTION' contains URL - diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R index 5786c1d1dd..19e088cb59 100644 --- a/tests/testthat/test-build.R +++ b/tests/testthat/test-build.R @@ -4,5 +4,5 @@ test_that("both versions of build_site have same arguments", { test_that("can build package without any index/readme", { pkg <- local_pkgdown_site(test_path("assets/site-empty")) - expect_output(build_site(pkg)) + expect_no_error(suppressMessages(build_site(pkg, new_process = FALSE))) }) diff --git a/tests/testthat/test-check-built.R b/tests/testthat/test-check-built.R new file mode 100644 index 0000000000..29a6170be5 --- /dev/null +++ b/tests/testthat/test-check-built.R @@ -0,0 +1,20 @@ +test_that("warn about missing images in readme", { + pkg <- local_pkgdown_site(test_path("assets/bad-images")) + suppressMessages(build_home(pkg)) + + expect_snapshot(check_built_site(pkg)) +}) + +test_that("readme can use images from vignettes", { + pkg <- local_pkgdown_site(test_path("assets/bad-images")) + file_copy( + test_path("assets/articles-images/man/figures/kitten.jpg"), + path(pkg$src_path, "vignettes/kitten.jpg") + ) + withr::defer(unlink(path(pkg$src_path, "vignettes/kitten.jpg"))) + + suppressMessages(build_home(pkg)) + suppressMessages(build_articles(pkg)) + + suppressMessages(expect_no_warning(check_built_site(pkg))) +}) diff --git a/tests/testthat/test-check.R b/tests/testthat/test-check.R index 4a85d4bca8..4d451f1fa5 100644 --- a/tests/testthat/test-check.R +++ b/tests/testthat/test-check.R @@ -1,46 +1,37 @@ -test_that("fails if reference index incomplete", { +test_that("sitrep reports all problems", { pkg <- local_pkgdown_site(test_path("assets/reference"), meta = " reference: - title: Title contents: [a, b, c, e] ") - expect_snapshot(check_pkgdown(pkg), error = TRUE) + expect_snapshot(pkgdown_sitrep(pkg)) }) - -test_that("fails if article index incomplete", { - pkg <- local_pkgdown_site(test_path("assets/articles"), meta = " - articles: +test_that("checks fails on first problem", { + pkg <- local_pkgdown_site(test_path("assets/reference"), meta = " + reference: - title: Title - contents: [starts_with('html'), random, standard, toc-false, widget, needs-escape] + contents: [a, b, c, e] ") expect_snapshot(check_pkgdown(pkg), error = TRUE) }) -test_that("informs if everything is ok", { - pkg <- local_pkgdown_site(test_path("assets/reference")) - expect_snapshot(check_pkgdown(pkg)) -}) - -# built site --------------------------------------------------------------- - -test_that("warn about missing images in readme", { - pkg <- local_pkgdown_site(test_path("assets/bad-images")) - suppressMessages(build_home(pkg)) - - expect_snapshot(check_built_site(pkg)) +test_that("both inform if everything is ok", { + pkg <- test_path("assets/open-graph") + expect_snapshot({ + pkgdown_sitrep(pkg) + check_pkgdown(pkg) + }) }) -test_that("readme can use images from vignettes", { - pkg <- local_pkgdown_site(test_path("assets/bad-images")) - file_copy( - test_path("assets/articles-images/man/figures/kitten.jpg"), - path(pkg$src_path, "vignettes/kitten.jpg") - ) - withr::defer(unlink(path(pkg$src_path, "vignettes/kitten.jpg"))) +# check urls ------------------------------------------------------------------ - suppressMessages(build_home(pkg)) - suppressMessages(build_articles(pkg)) +test_that("check_urls reports problems", { + # URL not in the pkgdown config + pkg <- test_path("assets/figure") + expect_snapshot(check_urls(pkg), error = TRUE) - suppressMessages(expect_no_warning(check_built_site(pkg))) + # URL only in the pkgdown config + pkg <- test_path("assets/cname") + expect_snapshot(check_urls(pkg), error = TRUE) }) diff --git a/tests/testthat/test-sitrep.R b/tests/testthat/test-sitrep.R deleted file mode 100644 index b710b89a8c..0000000000 --- a/tests/testthat/test-sitrep.R +++ /dev/null @@ -1,11 +0,0 @@ -test_that("pkgdown_sitrep works", { - # URL not in the pkgdown config - pkg <- test_path("assets/figure") - expect_snapshot(pkgdown_sitrep(pkg)) - # URL only in the pkgdown config - pkg <- test_path("assets/cname") - expect_snapshot(pkgdown_sitrep(pkg)) - # URL everywhere - pkg <- test_path("assets/open-graph") - expect_snapshot(pkgdown_sitrep(pkg)) -})