Prevent creation of faulty site #2411
Conversation
…in `build_reference_index()`.
|
Hmmmm, I was thinking more of a solution that would make this error disappear altogether. I don't currently remember enough about how pkgdown initialises a site, but I was thinking about automatically calling |
| cli::cli_abort(c( | ||
| "Can't create a site", | ||
| i = "Do you need to run {.run pkgdown::init_site()}?" | ||
| ), | ||
| call = call) |
There was a problem hiding this comment.
| cli::cli_abort(c( | |
| "Can't create a site", | |
| i = "Do you need to run {.run pkgdown::init_site()}?" | |
| ), | |
| call = call) | |
| init_site() |
I see. The problem is that init_site() is not very fast. Ideally, we would just call it when necessary. but I think that this may work?
And I think that the interactive condition has to remain there, otherwise this would break tests (or make them significantly slower)
There was a problem hiding this comment.
What if we (unconditionally) check for the presence of {path}/pkgdown.yml? My read of init_site() suggests that file will exist if init_site() has been run, and I doubt it will be created otherwise.
That will require some changes to the logic in is_non_pkgdown_site() too. Maybe to ignore all of the known top-level directories that pkgdown creates?
| @@ -247,6 +247,7 @@ examples_env <- function(pkg, seed = 1014, devel = TRUE, envir = parent.frame()) | |||
| #' @rdname build_reference | |||
| build_reference_index <- function(pkg = ".") { | |||
| pkg <- section_init(pkg, depth = 1L) | |||
| check_dst_path_exists(pkg$dst_path) | |||
| dir_create(path(pkg$dst_path, "reference")) | |||
There was a problem hiding this comment.
Looking at this again, it might be better to wrap check_...() and dir_create() into a new function like create_subdir() or something.
| cli::cli_abort(c( | ||
| "Can't create a site", | ||
| i = "Do you need to run {.run pkgdown::init_site()}?" | ||
| ), | ||
| call = call) |
There was a problem hiding this comment.
What if we (unconditionally) check for the presence of {path}/pkgdown.yml? My read of init_site() suggests that file will exist if init_site() has been run, and I doubt it will be created otherwise.
That will require some changes to the logic in is_non_pkgdown_site() too. Maybe to ignore all of the known top-level directories that pkgdown creates?
| check_dst_path_exists(pkg$dst_path) | ||
| dir_create(path(pkg$dst_path, "reference")) |
There was a problem hiding this comment.
| check_dst_path_exists(pkg$dst_path) | |
| dir_create(path(pkg$dst_path, "reference")) | |
| create_subdir(pkg$dst_path, "reference") |
To make sure I understand correctly:
if a pkgdown site: just create new reference dir, otherwise (if !fs::file_exists(pkg$dst_path, "pkgdown.yml")) call init_site() then create the new reference dir?
There was a problem hiding this comment.
Yeah, that's what I was thinking.
There was a problem hiding this comment.
Ok. I will think a little bit more. Will let you know when it is ready. I realized that this isn't an issue if you don't have _pkgdown.yml. You don't run in these issues with BS 3 for example..
Fix #2329
If you think the approach is reasonable, I could add this check in some strategic places.
For me, I'd see these as good candidates:
build_reference()build_home()build_articles()build_news()Add earlier message to prevent the creation of a faulty pkgdown site in
build_reference_index().I tested this interactively with no docs folder initialized and this works as expected.
Edit: the CI failure of no-pandoc appears genuine Maybe I could make the message conditional on the presence of pandoc? I added a condition on interactivity, but that seems a bit clunky.