Skip to content

Commit

Permalink
Unify config error messages (#2610)
Browse files Browse the repository at this point in the history
To always show path at the beginning, eliminating `config_path()` helper since it's no longer needed.
  • Loading branch information
hadley authored May 31, 2024
1 parent 11e1a8f commit 44f90ab
Show file tree
Hide file tree
Showing 25 changed files with 199 additions and 263 deletions.
2 changes: 1 addition & 1 deletion R/build-article.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ build_article <- function(name,

front <- rmarkdown::yaml_front_matter(input_path)
# Take opengraph from article's yaml front matter
front_opengraph <- check_open_graph(front$opengraph, input)
front_opengraph <- check_open_graph(pkg, front$opengraph, input)
data$opengraph <- modify_list(data$opengraph, front_opengraph)

# Allow users to opt-in to their own template
Expand Down
21 changes: 10 additions & 11 deletions R/build-news.R
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ build_news_multi <- function(pkg = ".") {

utils::globalVariables(".")

data_news <- function(pkg = list()) {
data_news <- function(pkg = list(), call = caller_env() ) {
html <- markdown_body(pkg, path(pkg$src_path, "NEWS.md"))
xml <- xml2::read_html(html)
downlit::downlit_html_node(xml)
Expand All @@ -163,14 +163,12 @@ data_news <- function(pkg = list()) {
xml2::xml_name()
ulevels <- unique(levels)
if (!identical(ulevels, "h1") && !identical(ulevels, "h2")) {
cli::cli_abort(
c(
"Invalid NEWS.md: inconsistent use of section headings.",
i = "Top-level headings must be either all <h1> or all <h2>.",
i = "See {.help pkgdown::build_news} for more details."
),
call = caller_env()
msg <- c(
"inconsistent use of section headings.",
i = "Top-level headings must be either all <h1> or all <h2>.",
i = "See {.help pkgdown::build_news} for more details."
)
config_abort(pkg, msg, path = "NEWS.md", call = call)
}
if (ulevels == "h1") {
# Bump every heading down a level so to get a single <h1> for the page title
Expand All @@ -183,10 +181,11 @@ data_news <- function(pkg = list()) {
sections <- sections[!is.na(versions)]

if (length(sections) == 0) {
cli::cli_warn(c(
"No version headings found in {src_path('NEWS.md')}",
msg <- c(
"no version headings found",
i = "See {.help pkgdown::build_news} for expected structure."
))
)
config_warn(pkg, msg, path = "NEWS.md", call = call)
}

versions <- versions[!is.na(versions)]
Expand Down
11 changes: 3 additions & 8 deletions R/check.R
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,14 @@ check_urls <- function(pkg = ".", call = caller_env()) {
url <- pkg$meta[["url"]]

if (is.null(url)) {
cli::cli_abort(
c("{config_path(pkg)} lacks {.field url}.", details),
call = call
)
config_abort(pkg, c("{.field url} is missing.", 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
)
msg <- "{.field URL} is missing package url ({url})."
config_abort(pkg, c(msg, details), path = "DESCRIPTION", call = call)
}
}
}
36 changes: 25 additions & 11 deletions R/config.R
Original file line number Diff line number Diff line change
Expand Up @@ -220,22 +220,36 @@ config_abort_type <- function(must_be, not, error_pkg, error_path, error_call) {

config_abort <- function(pkg,
message,
path = NULL,
...,
call = caller_env(),
.envir = caller_env()) {

message <- config_message(pkg, message, path)
cli::cli_abort(message, ..., call = call, .envir = .envir)
}

edit <- cli::format_inline("Edit {config_path(pkg)} to fix the problem.")

cli::cli_abort(
c(message, i = edit),
...,
call = call,
.envir = .envir
)
config_warn <- function(pkg,
message,
path = NULL,
...,
call = caller_env(),
.envir = caller_env()) {

message <- config_message(pkg, message, path)
cli::cli_warn(message, ..., call = call, .envir = .envir)
}

config_path <- function(pkg) {
config_message <- function(pkg, message, path = NULL) {
# Not all projects necessary have a _pkgdown.yml (#2542)
config <- pkgdown_config_path(pkg) %||% "_pkgdown.yml"
cli::style_hyperlink(path_file(config), paste0("file://", config))
path <- path %||% pkgdown_config_path(pkg) %||% "_pkgdown.yml"
if (is_absolute_path(path)) {
path_label <- path_rel(path, pkg$src_path)
} else {
path_label <- path
}

link <- cli::style_hyperlink(path_label, paste0("file://", path))
message[[1]] <- paste0("In ", link, ", ", message[[1]])
message
}
2 changes: 1 addition & 1 deletion R/package.R
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ get_bootstrap_version <- function(template, package = NULL, pkg) {
config_abort(
pkg,
c(
"Must set one only of {.field template.bootstrap} and {.field template.bslib.version}.",
"must set only one of {.field template.bootstrap} and {.field template.bslib.version}.",
i = hint
),
call = caller_env()
Expand Down
38 changes: 20 additions & 18 deletions R/render.R
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ data_template <- function(pkg = ".", depth = 0L) {
data_open_graph <- function(pkg = ".", call = caller_env()) {
pkg <- as_pkgdown(pkg)
og <- config_pluck_list(pkg, "template.opengraph", default = list())
og <- check_open_graph(og, pkgdown_config_path(pkg) %||% "_pkgdown.yml", call = call)
og <- check_open_graph(pkg, og, call = call)

logo <- find_logo(pkg$src_path)
if (is.null(og$image) && !is.null(logo)) {
Expand All @@ -161,44 +161,43 @@ data_open_graph <- function(pkg = ".", call = caller_env()) {
og
}

check_open_graph <- function(og, path, call = caller_env()) {
check_open_graph <- function(pkg, og, file_path = NULL, call = caller_env()) {
if (is.null(og)) {
return()
}

is_yaml <- path_ext(path) %in% c("yml", "yaml")
is_yaml <- is.null(file_path)
base_path <- if (is_yaml) "template.opengraph" else "opengraph"

check_open_graph_list(
pkg,
og,
file_path = path,
file_path = file_path,
error_path = base_path,
error_call = call
)

supported_fields <- c("image", "twitter")
unsupported_fields <- setdiff(names(og), supported_fields)
if (length(unsupported_fields)) {
cli::cli_warn(
"{.file {path}}: Unsupported {.field {base_path}} {cli::qty(unsupported_fields)} field{?s}: {.val {unsupported_fields}}.",
call = call
)
msg <- "{.field {base_path}} contains unsupported fields {.val {unsupported_fields}}."
config_warn(pkg, msg, path = file_path, call = call)
}
check_open_graph_list(
pkg,
og$twitter,
file_path = path,
file_path = file_path,
error_path = paste0(base_path, ".twitter"),
error_call = call
)
if (!is.null(og$twitter) && is.null(og$twitter$creator) && is.null(og$twitter$site)) {
cli::cli_abort(
"{.file {path}}: {.field opengraph.twitter} must include either {.field creator} or {.field site}.",
call = call
)
msg <- "{.field opengraph.twitter} must include either {.field creator} or {.field site}."
config_abort(pkg, msg, path = file_path, call = call)
}
check_open_graph_list(
pkg,
og$image,
file_path = path,
file_path = file_path,
error_path = paste0(base_path, ".image"),
error_call = call
)
Expand All @@ -213,16 +212,19 @@ render_template <- function(path, data) {
whisker::whisker.render(template, data)
}

check_open_graph_list <- function(x,
check_open_graph_list <- function(pkg,
x,
file_path,
error_path,
error_call = caller_env()) {
if (is.list(x) || is.null(x)) {
return()
}
not <- obj_type_friendly(x)
cli::cli_abort(
"{.file {file_path}}: {.field {error_path}} must be a list, not {not}.",
config_abort(
pkg,
"{.field {error_path}} must be a list, not {not}.",
path = file_path,
call = error_call
)
}
Expand Down
7 changes: 5 additions & 2 deletions R/theme.R
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,10 @@ get_bslib_theme <- function(pkg) {
check_bslib_theme(themes[[field]], pkg, field)
}

check_bslib_theme <- function(theme, pkg, field = "template.bootswatch", bs_version = pkg$bs_version) {
check_bslib_theme <- function(theme,
pkg,
field = "template.bootswatch",
bs_version = pkg$bs_version) {
bslib_themes <- c(
bslib::bootswatch_themes(bs_version),
bslib::builtin_themes(bs_version),
Expand All @@ -191,7 +194,7 @@ check_bslib_theme <- function(theme, pkg, field = "template.bootswatch", bs_vers
config_abort(
pkg,
c(
x = "Can't find Bootswatch/bslib theme preset {.val {theme}} ({.field {field}}).",
x = "{.field {field}} contains unknown Bootswatch/bslib theme {.val {theme}}.",
i = "Using Bootstrap version {.val {bs_version}} ({.field template.bootstrap})."
),
call = caller_env()
Expand Down
5 changes: 2 additions & 3 deletions tests/testthat/_snaps/build-article.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
build_rmarkdown_format(pkg, "article")
Condition
Error in `build_rmarkdown_format()`:
! code.width must be a whole number, not the string "abc".
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, code.width must be a whole number, not the string "abc".

# output is reproducible by default, i.e. 'seed' is respected

Expand All @@ -52,7 +51,7 @@
build_article(pkg = pkg, name = "bad-opengraph")
Condition
Error in `build_article()`:
! 'vignettes/bad-opengraph.Rmd': opengraph.twitter must be a list, not the number 1.
! In vignettes/bad-opengraph.Rmd, opengraph.twitter must be a list, not the number 1.

# render_rmarkdown copies image files in subdirectories

Expand Down
42 changes: 14 additions & 28 deletions tests/testthat/_snaps/build-articles.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,86 +4,73 @@
data_articles_index_(1)
Condition
Error in `data_articles_index_()`:
! articles must be a list, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, articles must be a list, not the number 1.
Code
data_articles_index_(list(1))
Condition
Error in `data_articles_index_()`:
! articles[1] must be a list, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, articles[1] must be a list, not the number 1.
Code
data_articles_index_(list(list()))
Condition
Error in `data_articles_index_()`:
! articles[1] must have components "title" and "contents".
! In _pkgdown.yml, articles[1] must have components "title" and "contents".
2 missing components: "title" and "contents".
i Edit _pkgdown.yml to fix the problem.
Code
data_articles_index_(list(list(title = 1, contents = 1)))
Condition
Error in `data_articles_index_()`:
! articles[1].title must be a string, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, articles[1].title must be a string, not the number 1.
Code
data_articles_index_(list(list(title = "a\n\nb", contents = 1)))
Condition
Error in `data_articles_index_()`:
! articles[1].title must be inline markdown.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, articles[1].title must be inline markdown.
Code
data_articles_index_(list(list(title = "a", contents = 1)))
Condition
Error in `data_articles_index_()`:
! articles[1].contents[1] must be a string.
! In _pkgdown.yml, articles[1].contents[1] must be a string.
i You might need to add '' around special YAML values like 'N' or 'off'
i Edit _pkgdown.yml to fix the problem.

# validates external-articles

Code
data_articles_(1)
Condition
Error in `data_articles_()`:
! external-articles must be a list, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, external-articles must be a list, not the number 1.
Code
data_articles_(list(1))
Condition
Error in `data_articles_()`:
! external-articles[1] must be a list, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, external-articles[1] must be a list, not the number 1.
Code
data_articles_(list(list(name = "x")))
Condition
Error in `data_articles_()`:
! external-articles[1] must have components "name", "title", "href", and "description".
! In _pkgdown.yml, external-articles[1] must have components "name", "title", "href", and "description".
3 missing components: "title", "href", and "description".
i Edit _pkgdown.yml to fix the problem.
Code
data_articles_(list(list(name = 1, title = "x", href = "x", description = "x")))
Condition
Error in `data_articles_()`:
! external-articles[1].name must be a string, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, external-articles[1].name must be a string, not the number 1.
Code
data_articles_(list(list(name = "x", title = 1, href = "x", description = "x")))
Condition
Error in `data_articles_()`:
! external-articles[1].title must be a string, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, external-articles[1].title must be a string, not the number 1.
Code
data_articles_(list(list(name = "x", title = "x", href = 1, description = "x")))
Condition
Error in `data_articles_()`:
! external-articles[1].href must be a string, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, external-articles[1].href must be a string, not the number 1.
Code
data_articles_(list(list(name = "x", title = "x", href = "x", description = 1)))
Condition
Error in `data_articles_()`:
! external-articles[1].description must be a string, not the number 1.
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, external-articles[1].description must be a string, not the number 1.

# articles in vignettes/articles/ are unnested into articles/

Expand All @@ -99,6 +86,5 @@
. <- data_articles_index(pkg)
Condition
Error:
! 1 vignette missing from index: "c".
i Edit _pkgdown.yml to fix the problem.
! In _pkgdown.yml, 1 vignette missing from index: "c".

Loading

0 comments on commit 44f90ab

Please sign in to comment.