Skip to content

Commit

Permalink
Unify sitrep and check (r-lib#2516)
Browse files Browse the repository at this point in the history
* Introduce new `check_urls()` function that reports on url problems with errors and call it in `check_pkgdown()`
* `pkgdown_sitrep()` now calls the same functions as `check_pkgdown()` but wrapped in a helper to convert errors to messages
* `check_built()` is a separate concern so it's moved to a separate file
* Improve articles checking by reusing reference checks.

Fixes r-lib#2463. Fixes r-lib#2380.
  • Loading branch information
hadley authored and SebKrantz committed Jun 1, 2024
1 parent dd353aa commit a794973
Show file tree
Hide file tree
Showing 16 changed files with 205 additions and 198 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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).
Expand Down
16 changes: 6 additions & 10 deletions R/build-articles.R
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>%
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions R/build-reference-index.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions R/build.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions R/check-built.R
Original file line number Diff line number Diff line change
@@ -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}"
))
}
}
85 changes: 61 additions & 24 deletions R/check.R
Original file line number Diff line number Diff line change
@@ -1,50 +1,87 @@
#' 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)

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
)
}
}
}
47 changes: 0 additions & 47 deletions R/sitrep.R

This file was deleted.

19 changes: 13 additions & 6 deletions man/check_pkgdown.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 0 additions & 18 deletions man/pkgdown_sitrep.Rd

This file was deleted.

11 changes: 11 additions & 0 deletions tests/testthat/_snaps/check-built.md
Original file line number Diff line number Diff line change
@@ -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'

59 changes: 40 additions & 19 deletions tests/testthat/_snaps/check.md
Original file line number Diff line number Diff line change
@@ -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)`.

Loading

0 comments on commit a794973

Please sign in to comment.