Use config helpers consistently #2556
Conversation
|
@jayhesselberth @maelle no need for a detailed review, but if you see anything that looks amiss, please let me know. This should substantially improve the errors we give people when they make mistakes in yaml. I'm sure it doesn't yet catch every possible failure mode, but I think it gives us a good path to follow for the future. |
| data_articles_index_(list(1)) | ||
| Condition | ||
| Error in `data_articles_index_()`: | ||
| ! articles[1] must be a list, not the number 1. |
There was a problem hiding this comment.
Do you think it's reasonably clear that I'm talking about the first component of articles here?
There was a problem hiding this comment.
Why not write it out and put articles as code
Component 1 of articles must be a list, not the number 1.
In any case it's helpful that the last line of the error refers to the config.
There was a problem hiding this comment.
I'm not convinced that spelling it out in words will help. I think the mapping between the R data structures and the yaml structure is likely to be confusing, but I don't think that there's much better we can do until RStudio supports feedback directly on the yaml file (which I think is likely to come by the end of the year).
I do wonder if it would be better to put the file name closer, e.g. _pkgdown.yml: articles[1] must be a list.... (Making _pkgdown.yml a link so we can remove the edit bullet at the end).
|
|
||
| articles <- pkg$vignettes$file_out[is_vig_in_articles] | ||
| purrr::map(articles, ~ paste0(c("articles/", ""), .x)) | ||
| data_redirects <- function(pkg, has_url = FALSE, call = caller_env()) { |
There was a problem hiding this comment.
Introduced new data_redirects, like most of the other components, to make testing easier.
| @@ -100,15 +100,15 @@ find_tutorials <- function(path = ".") { | |||
| } | |||
|
|
|||
| tutorial_info <- function(path, base_path) { | |||
| meta <- rmarkdown::yaml_front_matter(path) | |||
| title <- meta$title | |||
| yaml <- rmarkdown::yaml_front_matter(path) | |||
There was a problem hiding this comment.
Renamed this in order to reserve meta for site metadata.
| "{.field {where}} must supply an inline element, not a block element.", | ||
| call = caller_env() | ||
| error_pkg, | ||
| "{.field {error_path}} must be inline markdown.", |
There was a problem hiding this comment.
I don't love this error, but I think it's a bit clearer than the previous one (since it at least mentions markdown).
| data_articles_index_(list(1)) | ||
| Condition | ||
| Error in `data_articles_index_()`: | ||
| ! articles[1] must be a list, not the number 1. |
There was a problem hiding this comment.
Why not write it out and put articles as code
Component 1 of articles must be a list, not the number 1.
In any case it's helpful that the last line of the error refers to the config.
| prefix <- config_pluck_string( | ||
| pkg, | ||
| "authors.footer.text", | ||
| default = tr_("Developed by"), |
There was a problem hiding this comment.
| default = tr_("Developed by"), | |
| default = tr_("Developped by"), |
There was a problem hiding this comment.
It sounds like that's how you should spell it, but I'm pretty sure my spelling is correct here 😄
| component <- components[[name]] | ||
| component_path <- paste0(base_path, ".", name) | ||
|
|
||
| config_pluck_list(pkg, component_path, has_names = c("title", "text"), call = call) |
There was a problem hiding this comment.
I do not understand this and the following two lines. they're called only to check the contents of components, is that correct?
There was a problem hiding this comment.
Yeah, that's right. It's a tiny bit sloppy, but it's easier than writing another function that only check, but takes the same specification as config_pluck_.
That said, I'm not 100% happy with this API, so it might change again a bit in the future (but not in this PR).
* Add `config_check_bool()` and use it for news * Validate components of home * Check footer components * Switch to allowing `NULLs` since all defaults are effectively `NULL` * Switch redirects checking to new style * And update home index * Check `width` * Update articles and reference checking * Update home sidebar code * Add news bullet * Drop accidentally added test * Fix missing call * Remove unusued function * Align `markdown_text_inline()` error arguments and ... * Use `modify_list()`
Fixes #1927