Skip to content

Commit

Permalink
Check readme images after build refrence and articles
Browse files Browse the repository at this point in the history
We can't know if the images in a readme exist until we have built the rest of the site. We worked around this a bit for images in `man/figures` but that strategy doesn't work for articles since we actually need to build them. So now we just check in a new step at the end of `build_site()`.

Fixes #2194
  • Loading branch information
hadley committed Apr 24, 2024
1 parent 4e7ffec commit ffc11eb
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 51 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# pkgdown (development version)

* `build_home()` no longer checks if the README is missing any images. This check is now performed in `build_site()`, after `build_articles()` so you can refer to images created by vignettes with warnings (#2194).
* Very wide words are now automatically broken across lines and hyphenated (when possible) when they'd otherwise create a horizontal scrollbar on mobile (#1888).
* The `repo.source.url` field no longer requires a trailing slash (#2017).
* Anywhere you can use `_pkgdown.yml`, you can now use `_pkgdown.yaml` (#2244).
Expand Down
39 changes: 11 additions & 28 deletions R/build-home-index.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,7 @@
build_home_index <- function(pkg = ".", quiet = TRUE) {
pkg <- section_init(pkg, depth = 0L)

src_path <- path_first_existing(
pkg$src_path,
c("pkgdown/index.md",
"index.md",
"README.md"
)
)
src_path <- path_index(pkg)
dst_path <- path(pkg$dst_path, "index.html")
data <- data_home(pkg)

Expand All @@ -34,12 +28,19 @@ build_home_index <- function(pkg = ".", quiet = TRUE) {
logo = logo_path(pkg, depth = 0)
)

copy_figures(pkg)
check_missing_images(pkg, path_rel(src_path, pkg$src_path), "index.html")

invisible()
}

path_index <- function(pkg) {
path_first_existing(
pkg$src_path,
c("pkgdown/index.md",
"index.md",
"README.md"
)
)
}

data_home <- function(pkg = ".") {
pkg <- as_pkgdown(pkg)

Expand Down Expand Up @@ -215,21 +216,3 @@ cran_link <- memoise(function(pkg) {

NULL
})


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 {src_path}}: {.file {paths}}",
i = "pkgdown can only use images in {.file man/figures} and {.file vignettes}"
))
}
}
2 changes: 2 additions & 0 deletions R/build.R
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,8 @@ build_site_local <- function(pkg = ".",
build_search(pkg, override = override)
}

check_built_site(pkg)

cli::cli_rule("Finished building pkgdown site for package {.pkg {pkg$package}}")
preview_site(pkg, preview = preview)
}
27 changes: 27 additions & 0 deletions R/check.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,30 @@ check_pkgdown <- function(pkg = ".") {
"v" = "No problems found in {pkgdown_config_href({pkg$src_path})}"
))
}

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}"
))
}
}
2 changes: 1 addition & 1 deletion R/rmarkdown.R
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ render_rmarkdown <- function(pkg, input, output, ..., seed = NULL, copy_images =
dir_create(unique(path_dir(dst)))
file_copy(src, dst, overwrite = TRUE)
}
check_missing_images(pkg, input, output)
check_missing_images(pkg, input_path, output)

invisible(path)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/_snaps/build-articles.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
Writing `articles/html-vignette.html`
Condition
Warning:
Missing images in 'vignettes/html-vignette.Rmd': 'foo.png'
Missing images in 'vignettes/html-vignette.Rmd': 'kitten.jpg'
i pkgdown can only use images in 'man/figures' and 'vignettes'

# articles don't include header-attrs.js script
Expand Down
14 changes: 0 additions & 14 deletions tests/testthat/_snaps/build-home.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,6 @@
Writing `authors.html`
Writing `404.html`

# warns about missing images

Code
build_home(pkg)
Message
-- Building home ---------------------------------------------------------------
Writing `authors.html`
Condition
Warning:
Missing images in 'README.md': 'foo.png'
i pkgdown can only use images in 'man/figures' and 'vignettes'
Message
Writing `404.html`

# can build site even if no Authors@R present

Code
Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/_snaps/check.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,21 @@
Message
v No problems found in _pkgdown.yml

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

# readme can use images from vignettes

Code
check_built_site(pkg)
Message
-- Checking for problems -------------------------------------------------------

2 changes: 1 addition & 1 deletion tests/testthat/assets/bad-images/README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@

![](foo.png)
![](vignettes/kitten.jpg)
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ title: "test"
knitr::opts_chunk$set(collapse = TRUE, comment = "#>")
```

![](foo.png)
![](kitten.jpg)
5 changes: 0 additions & 5 deletions tests/testthat/test-build-home.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ test_that("intermediate files cleaned up automatically", {
)
})

test_that("warns about missing images", {
pkg <- local_pkgdown_site(test_path("assets/bad-images"))
expect_snapshot(build_home(pkg))
})

test_that("can build site even if no Authors@R present", {
skip_if_no_pandoc()

Expand Down
23 changes: 23 additions & 0 deletions tests/testthat/test-check.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,26 @@ 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("readme can use images from vignettes", {
pkg <- local_pkgdown_site(test_path("assets/bad-images"))
file_copy(
test_path("assets/man-figures/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))

expect_snapshot(check_built_site(pkg))
})

0 comments on commit ffc11eb

Please sign in to comment.