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

Fix typo in vignette #2557

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Fix typo in vignette #2557

merged 1 commit into from
Jun 6, 2024

Conversation

olivroy
Copy link
Collaborator

@olivroy olivroy commented May 18, 2024

Minor typo,

But while I was reading the vignette, some thoughts occured to me. Just thought I'd share them as the timing seems right (actively working on pkgdown 2.1, and closed more than 100 issues in a few weeks(?!) you are a machine!)

I wanted to share my thoughts on my last pain point when iterating and customizing a site locally: init_site() or no init_site().

typical site customization workflow

In this type of workflow, now allowed with #2439 and #2571, I find step 0.1 to be the most nebulous

  1. usethis::use_pkgdown()
    0.1 init_site() (only needed sometimes?)
  2. Edit _pkgdown.yml or other site components (adding a logo, color, changing font, layout etc.)
  3. build_home()
  4. Preview page
  5. Repeat (1-3)

In short, I'd want to know what special edits to site config (1) requires manually running init_site() (0.1).

I would use your answer to add this to ?init_site, as it seems sufficient. BUT, ideally, it would be great to identify these special cases, if there was a way for pkgdown to be smart about this, therefore acheiving the end goal I had when I opened #2439 and #2329.

You don't do site customization this often, but when you do, it is more fun if it's not too difficult!

In very long words, my unfiltered thoughts on init_site()

When iterating on theming or customizing a site, sometimes you need to run init_site(), (which can take a longer) (especially on Windows with no SSD, copying files is sooo slow, I don't understand why). So, it is slow to call this a lot.

I see that in the vignette, it is required to call init_site() when changing font for previewing. I guess it is required in other contexts, what are they? And when is it not required? AFAIR, iterating by changing bslib variables in template doesn't require init_site().

I wonder if you could add to ?init_site a section like that.

When do you need to run init_site().

  • When iterating on theme.css?
  • When changing fonts

When you don't have to (since we made that redundant in #2439

  • to create the site: call usethis::use_pkgdown() + build_site() instead.
  • To preview site: use build_site().
  • when changing bslib variables in _pkgdown.yml

Maybe a more agile refresh_site() or init_site(lazy = TRUE) could be introduced that would only do the required things? (a la usethis::use_github_action(), Calling use_github_action() asks (or lets you and informs) overwrite R-CMD-check.yml.
Personally, I am leveraging usethis::write_over() in my workflows. I wonder if pkgdown could have something similar.

AFAICT, init_site(lazy = TRUE) would have to do something if.

  • if docs/ doesn't exist
    • regular init_site()
  • if _pkgdown.yml changed
    • Copy it if changed
    • if detected a change in special components, call init_site(lazy = FALSE)
  • If logo has changed,
    • refresh_site() would know about this and would inform + overwrite logo favicons. Otherwise, does nothing.
  • If theme.css has changed
    • copy it to docs/
  • if development.mode changes?

Since pkgdown keeps a copy of pkgdown.yml in the docs/ folder, it would just be to a few targeted comparisons, between _pkgdown.yml and docs/_pkgdown.yml

Do inform of how init_site(lazy = TRUE) handled special components.

  • Kept logo unchanged
  • Kept theme.css unchanged
  • Kept fonts unchanged
    etc.

Here are my thoughts on init_site() for context.

(Disclaimer: I haven't followed all your latest development on pkgdown after 2.0.9 release) I opened, init_site() was the main friction point left for me, as I don't like calling it (I find it long to run, and I am never sure if it really has to do all the things it does, what it overwrites and

Overall, it is just not clear to me when you have to build everything from scratch, and when things are fine and you can preview your site, and it will show your recent additions.

I ended up calling unlink("docs/", recursive = TRUE) way too many times, as I didn't know

@hadley
Copy link
Member

hadley commented May 20, 2024

You need to call init_site() whenever build_bslib() needs to be re-run. And it's a bit hard for pkgdown to tell when this is because it's bslib doing all the computation — but at a minimum it's whenever you modify anything in template.bslib, or you modify extra.scss file, or when a template changes it, or when bslib itself changes it. Figuring that out is going to be hard, so I think a better angle of attack would be to figure out why init_site() is so slow for you, and fix that. Does profvis give any hint at where the slowness lies?

@olivroy
Copy link
Collaborator Author

olivroy commented Jun 6, 2024

Thanks for the explanations!

You need to call init_site() whenever build_bslib() needs to be re-run

build_bslib() takes about half the time 2.3 seconds, but it is not something pkgdown can control (profiling suggests sass compilation is the thing that takes time)

But maybe something that can be controlled is as_pkgdown():
The parseRd() in package_topics() takes half of the total time. It seems to run only once, so that's good. I am not seeing a way that as_pkgdown() could be "cached" even more.

I found however something where I could provide a small improvement: cli message printing!

I am sharing a PR shortly to reduce use of readCitationFile() r-lib/cli#607 to reduce time it takes to print output. sass takes a bit of time,

f <- function() {
  cli::cli_inform("text")
  cli::cli_inform("text")
  cli::cli_inform("text")
  cli::cli_inform("text")
}

g <- function() {
  # Computing number of colors once for speed r-lib/cli#607
  withr::local_options(cli.num_colors = cli::num_ansi_colors())
  cli::cli_inform("text")
  cli::cli_inform("text")
  cli::cli_inform("text")
  cli::cli_inform("text")
}
bench::mark(
  f(),
  g()
)
#> # A tibble: 2 × 6
#>   expression      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 f()          33.2ms   36.8ms      25.3    6.52MB     8.44
#> 2 g()          33.3ms   40.9ms      21.2   66.17KB     7.07

Created on 2024-06-06 with reprex v2.1.0

I use this trick in my package and .Rprofile where I print things to console repeatedly and it helps!

Overall, it takes 5 seconds to run init_site() on pkgdown itself.

Follow-up question:

but at a minimum it's whenever you modify anything in template.bslib, or you modify extra.scss

I am guessing this includes extra.css too? Also when / if you modify pkgdown/assets? (I will try to document this)

Overall file copying takes around 0.7 seconds from what I saw

@olivroy
Copy link
Collaborator Author

olivroy commented Jun 6, 2024

Will merge this since this is only a little typo correction :)

@olivroy olivroy merged commit 937878b into r-lib:main Jun 6, 2024
15 checks passed
@olivroy olivroy deleted the patch-1 branch June 6, 2024 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants