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

Don't call init_site() when lazy = TRUE + use posix line end + document init_site() + speed cli console output #2642

Merged
merged 11 commits into from
Jun 7, 2024
6 changes: 5 additions & 1 deletion R/build.R
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,11 @@ build_site_local <- function(pkg = ".",

pkgdown_sitrep(pkg)

init_site(pkg)
if (!lazy) {
# Only force init_site() if `!lazy`
# if site is not initialized, it will be in build_home()
init_site(pkg)
}

build_home(pkg, override = override, preview = FALSE)
build_reference(
Expand Down
8 changes: 8 additions & 0 deletions R/context.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,18 @@ section_init <- function(pkg, depth, override = list(), .frame = parent.frame())
rstudio_save_all()
local_envvar_pkgdown(pkg, .frame)
local_options_link(pkg, depth = depth, .frame = .frame)
local_options_console(.frame = .frame)

pkg
}

local_options_console <- function(.frame = parent.frame()) {
olivroy marked this conversation as resolved.
Show resolved Hide resolved
# Improves speed of init_site() with tip from r-lib/cli#607
# Only does so if no option set
olivroy marked this conversation as resolved.
Show resolved Hide resolved
num_col <- getOption("cli.num_colors", default = cli::num_ansi_colors())
withr::local_options(cli.num_colors = num_col, .local_envir = .frame)
}

local_options_link <- function(pkg, depth, .frame = parent.frame()) {
article_index <- article_index(pkg)
Rdname <- get_rdname(pkg$topics)
Expand Down
9 changes: 9 additions & 0 deletions R/init.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
#' * copies CSS/JS assets and extra files, and
#' * runs `build_favicons()`, if needed.
#'
#' Typically, you will not need to call this function directly, as all `build_*()`
#' functions will run `init_site()` if needed.
#'
#' The only good reasons to call `init_site()` directly are the following:
#' * If you add or modify a package logo.
#' * If you add or modify `pkgdown/extra.scss`.
#' * If you modify `template.bslib` variables in `_pkgdown.yml`.
#'
#' See `vignette("customise")` for the various ways you can customise the
#' display of your site.
#'
Expand All @@ -21,6 +29,7 @@
#' @inheritParams build_articles
#' @export
init_site <- function(pkg = ".") {
local_options_console()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure we shouldn't just callsection_init() here? I also noticed that init_site() lacks override, and I needed it when I started experimenting with quarto.

That would make it more consistent with the other functions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now clear that it shouldn't be called here 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Hadley, actually, cache_cli_colours() should be used in init_site(),

bench marks

# without caching
 expression    min median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result
  <bch:expr>  <bch> <bch:>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list>
1 init_site() 3.23s  3.23s     0.309    14.7MB    0.619     1     2      3.23s <NULL>
# with caching
  expression    min median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result
  <bch:expr>  <bch> <bch:>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list>
1 init_site() 3.02s  3.02s     0.331    8.89MB    0.992     1     3      3.02s <NULL>

Significant difference in memory usage. And also, the output prints much faster in the console!

pkg <- as_pkgdown(pkg)

if (is_non_pkgdown_site(pkg$dst_path)) {
Expand Down
2 changes: 2 additions & 0 deletions R/theme.R
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ build_bslib <- function(pkg = ".", call = caller_env()) {

data_deps <- function(pkg, depth) {
if (!file_exists(data_deps_path(pkg))) {
# this is unlikely to occur after #2439 and #2571
cli::cli_abort(
"Run {.fn pkgdown::init_site} first.",
.internal = TRUE,
call = caller_env()
)
}
Expand Down
10 changes: 10 additions & 0 deletions man/init_site.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 24 additions & 23 deletions pkgdown.Rproj
olivroy marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
Version: 1.0

RestoreWorkspace: Default
SaveWorkspace: Default
AlwaysSaveHistory: Default

EnableCodeIndexing: Yes
UseSpacesForTab: Yes
NumSpacesForTab: 2
Encoding: UTF-8

RnwWeave: Sweave
LaTeX: pdfLaTeX

AutoAppendNewline: Yes
StripTrailingWhitespace: Yes

BuildType: Package
PackageUseDevtools: Yes
PackageInstallArgs: --no-multiarch --with-keep.source
PackageRoxygenize: rd,collate,namespace

SpellingDictionary: en_AU
Version: 1.0

RestoreWorkspace: No
SaveWorkspace: No
AlwaysSaveHistory: Default

EnableCodeIndexing: Yes
UseSpacesForTab: Yes
NumSpacesForTab: 2
Encoding: UTF-8

RnwWeave: Sweave
LaTeX: pdfLaTeX

AutoAppendNewline: Yes
StripTrailingWhitespace: Yes
LineEndingConversion: Posix

BuildType: Package
PackageUseDevtools: Yes
PackageInstallArgs: --no-multiarch --with-keep.source
PackageRoxygenize: rd,collate,namespace

SpellingDictionary: en_AU
Loading