Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn if in home and articles don't exist #1810

Merged
merged 3 commits into from
Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# pkgdown (development version)

* `build_articles()` and `build_home()` now warn if you have images that
won't rendered on the website because they're in unsupported directories
(#1810). Generally, it's only safe to refer to figures in `man/figures`
and `vignettes`.

* Auto-generated links to inherited R6 methods now work correctly
whether internal (#1173, @vandenman) or external (#1476).

Expand Down
20 changes: 20 additions & 0 deletions R/build-home-index.R
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ build_home_index <- function(pkg = ".", quiet = TRUE) {
logo = logo_path(pkg, depth = 0)
)

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

invisible()
}

Expand Down Expand Up @@ -199,3 +201,21 @@ 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 <- encodeString(rel_src[!exists], quote = "'")
warn(c(
paste0("Missing images in '", src_path, "': ", paste0(paths, collapse = ", ")),
i = "pkgdown can only use images in 'man/figures' and 'vignettes'"
))
}
}
9 changes: 6 additions & 3 deletions R/build-home-md.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ build_home_md <- function(pkg) {

render_md <- function(pkg, filename) {
body <- markdown_body(filename, strip_header = TRUE, pkg = pkg)
path <- path_ext_set(basename(filename), "html")

cat_line("Reading ", src_path(path_rel(filename, pkg$src_path)))

Expand All @@ -33,11 +34,13 @@ render_md <- function(pkg, filename) {
filename = filename,
source = repo_source(pkg, fs::path_rel(filename, pkg$src_path))
),
path = path_ext_set(basename(filename), "html")
path = path
)

if (basename(filename) == "404.md") {
update_html(path_abs("404.html", start = pkg$dst_path), tweak_link_absolute, pkg = pkg)
if (path == "404.html") {
update_html(path(pkg$dst_path, path), tweak_link_absolute, pkg = pkg)
}
check_missing_images(pkg, filename, path)

invisible()
}
1 change: 1 addition & 0 deletions R/rmarkdown.R
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ render_rmarkdown <- function(pkg, input, output, ..., copy_images = TRUE, quiet
dir_create(unique(path_dir(dst)))
file_copy(src, dst, overwrite = TRUE)
}
check_missing_images(pkg, input, output)

invisible(path)
}
Expand Down
13 changes: 13 additions & 0 deletions tests/testthat/_snaps/build-articles.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# warns about missing images

Code
build_articles(pkg)
Output
-- Building articles -----------------------------------------------------------
Writing 'articles/index.html'
Reading 'vignettes/html-vignette.Rmd'
Writing 'articles/html-vignette.html'
Warning <rlang_warning>
Missing images in 'vignettes/html-vignette.Rmd': 'foo.png'
i pkgdown can only use images in 'man/figures' and 'vignettes'

11 changes: 11 additions & 0 deletions tests/testthat/_snaps/build-home.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# warns about missing images

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

9 changes: 9 additions & 0 deletions tests/testthat/assets/bad-images/DESCRIPTION
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Package: testpackage
Version: 1.0.0
Title: A test package
Description: A longer statement about the package.
Authors@R: c(
person("Hadley", "Wickham", , "hadley@rstudio.com", role = c("aut", "cre")),
person("RStudio", role = c("cph", "fnd"))
)
RoxygenNote: 6.0.1
2 changes: 2 additions & 0 deletions tests/testthat/assets/bad-images/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

![](foo.png)
9 changes: 9 additions & 0 deletions tests/testthat/assets/bad-images/vignettes/html-vignette.Rmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
title: "test"
---

```{r include = FALSE}
knitr::opts_chunk$set(collapse = TRUE, comment = "#>")
```

![](foo.png)
2 changes: 1 addition & 1 deletion tests/testthat/assets/sidebar/README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

<!-- README.md is generated from README.Rmd. Please edit that file -->

# pkgdown <img src="man/figures/logo.png" align="right" alt="" width="120" />
# pkgdown

<!-- badges: start -->

Expand Down
15 changes: 10 additions & 5 deletions tests/testthat/test-build-articles.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ test_that("can recognise intro variants", {
})

test_that("links to man/figures are automatically relocated", {
pkg <- test_path("assets/man-figures")
dst <- withr::local_tempdir()
pkg <- local_pkgdown_site(test_path("assets/man-figures"))

expect_output(build_articles(pkg, override = list(destination = dst)))
expect_output(copy_figures(pkg))
expect_output(build_articles(pkg, lazy = FALSE))

html <- xml2::read_html(path(dst, "articles", "kitten.html"))
html <- xml2::read_html(path(pkg$dst_path, "articles", "kitten.html"))
src <- xpath_attr(html, "//img", "src")

expect_equal(src, c(
Expand All @@ -22,7 +22,12 @@ test_that("links to man/figures are automatically relocated", {
))

# And files aren't copied
expect_false(dir_exists(path(dst, "man")))
expect_false(dir_exists(path(pkg$dst_path, "man")))
})

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

test_that("articles don't include header-attrs.js script", {
Expand Down
5 changes: 5 additions & 0 deletions tests/testthat/test-build-home.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ 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