rstudio / blogdown

Create Blogs and Websites with R Markdown
https://pkgs.rstudio.com/blogdown/
1.74k stars 330 forks source link

Improve error/warning messages #510

Closed apreshill closed 3 years ago

apreshill commented 4 years ago

After running into many error/warning messages with v0.21.42, I think we can improve these to make the user experience better and make the message more readable and actionable cc @cderv

For example, I got this error:

The 'publish' setting in 'netlify.toml' is 'public/' but the publish dir in the Hugo config file is 'public'. You may want to change the former to the latter.

The printing and phrasing of this messages (in addition to the volume of messages that get printed to my console when I serve the site) makes it hard to understand what is being checked, compared, and what I should do to make the warning go away. It would be nice if the printed message were more user-friendly and formatted nicely in the console, like usethis messages that use cli and crayon. Something like this:

✓ Checking `netlify.toml` file, set 'publish = docs' 
✓ Checking `config.toml` file, set 'publishDir = publish' 
✓ Comparing `netlify.toml` and `config.toml` publish directories
● Warning: publish directories do not match. We recommend editing 
your `config.toml` file to set 'publishDir = docs'.  

Similarly:

✓ Checking `blogdown::hugo_version()`, set as `0.71.1`
✓ Checking `netlify.toml` file, set `HUGO_VERSION = 0.65.0`
✓ Comparing `blogdown` and `netlify.toml` Hugo versions
● Warning: Hugo versions do not match. We recommend editing 
your `netlify.toml` file to set `HUGO_VERSION = 0.71.1`. 
If you do not wish to edit the `netlify.toml` file, you may instead use 
`blogdown::install_hugo("0.65.0")` to change your local Hugo version.

And for Hugo installations, I see:

Warning message: Found hugo at "/Users/alison/Library/Application Support/Hugo/0.71.1/hugo" and "/usr/local/bin/hugo". The former will be used. If you don't need both copies, you may delete/uninstall the latter.

It would be great if it looked more like:

✓ Checking local Hugo installations
✓ Copy #1 found at "/Users/alison/Library/Application Support/Hugo/0.71.1/hugo"
✓ Copy #2 found at "/usr/local/bin/hugo"
● Warning: multiple Hugo installations found; using copy #1. 
If you don't need both copies, you may uninstall copy #2 with _______.

Also, this message always prints twice for me:

> blogdown:::serve_site()
--------------------------------------------------------------------------------
You are recommended to ignore more items in the 'ignoreFiles' field in config.toml: "\\.Rmd$", "\\.Rmarkdown$", "\\.knit\\.md$", "\\.utf8\\.md$"
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
You are recommended to remove the item '_files$' in the 'ignoreFiles' field in config.toml.
--------------------------------------------------------------------------------

This is also quite long, I wonder if we could wrap in an argument to serve_site like verbose that defaults to FALSE?:

Launching the server via the command:
  /Users/alison/Library/Application Support/Hugo/0.71.1/hugo server --bind 127.0.0.1 -p 4321 --themesDir themes -t hugo-academic -D -F --navigateToChanged
Serving the directory . at http://127.0.0.1:4321

By filing an issue to this repo, I promise that

spcanelon commented 3 years ago

Hello! 👋

I recently had a related and very confusing experience with blogdown and wanted to pop in and express my support for something like what @apreshill described above:

make the [error/warning messages] more readable and actionable

Having more readable and actionable messages would be incredibly helpful.

For my issue I had overlooked the error/warning messages when I went back to update a blog post with some text and links. When my post was rendered, the embedded xaringan slides embedded were oversized and my first thought was that the xaringanExtra package maybe had a bug, so I reached out to @gadenbuie.

Garrick was kind enough to help me identify the issue had to do with a JavaScript file used by xaringanExtra::share_again() and stored in the index_files folder of my post. What was harder for me to figure out was that this message (referenced by Alison above) was in some way related:

--------------------------------------------------------------------------------
You are recommended to remove the item '_files$' in the 'ignoreFiles' field in config.toml.
--------------------------------------------------------------------------------

The language in this message is also a bit passive which didn't help me grasp that there would/could be consequences of not following the recommendation 😅

I know the blogdown team is working hard to create a sort of handy pre-flight checklist of functions to help users catch these things before their site breaks 👏 and I think having more readable and actionable messages aligns nicely with those user experience goals. These types of messages would make a big difference for users like me that update their blog every couple of months and don't necessarily keep up with blogdown news.

Lastly, I think more readable/actionable messages would help make the implicit processes a little more explicit which is a great educational bonus. I know it would help me learn more about what's going on behind the scenes and feel more confident about exploring new-to-me features in blogdown 😃

yihui commented 3 years ago

@spcanelon I've been focusing on the improvement of these messages for several days (in the new blogdown::check_site() function in the current dev version of blogdown), and I really appreciate your timely feedback! Thank you so much!

spcanelon commented 3 years ago

Hi @yihui, I appreciate you dedicating time and effort to improving these messages! And I'm glad I was able to share my experience as a user at a relevant time, thanks so much for taking it into account 🙂

yihui commented 3 years ago

@spcanelon I just merged the PR #534 to address this issue. You can install the development version via

remotes::install_github('rstudio/blogdown')

and see if messages from blogdown::check_site() are more helpful and clear now. Thanks!

spcanelon commented 3 years ago

Hi @yihui...let me just say: wow. Thanks so much to you, @apreshill, and @cderv for all of your hard work on this! 👏

This is what the message looked like before:

--------------------------------------------------------------------------------
You are recommended to ignore more items in the 'ignoreFiles' field in config.toml: "\\.knit\\.md$", "\\.utf8\\.md$"
--------------------------------------------------------------------------------

And now after PR #534, I see this wonderful, readable, and actionable message for config.toml when I run blogdown::check_site():

― Checking config.toml
| Checking "baseURL" setting for Hugo...
○ Found baseURL = "https://silvia.rbind.io/"; nothing to do here!
| Checking "ignoreFiles" setting for Hugo...
● [TODO] Add these items to the "ignoreFiles" setting: "\\.knit\\.md$", "\\.utf8\\.md$"
| Checking setting for Hugo's Markdown renderer...
○ All set! Found the "unsafe" setting for goldmark.
― Check complete: config.toml

I can understand exactly what's being checked, I'm able to follow along with what's going on behind the scenes with each step, and I understand what I need to do next. What a difference! These messages are so much more helpful and clear -- I am so impressed!

Also, the fact that config.toml was also automatically opened for me was a nice bonus. It helped save me the time it would take to locate the file in my directory.

blogdown::check_site() revealed a bunch of other [TODO] items for me to examine and with this new formatting I don't feel intimidated! I'm even looking forward to diving in and fixing all of the things 😆

Thanks again!

yihui commented 3 years ago

Also, the fact that config.toml was also automatically opened for me was a nice bonus. It helped save me the time it would take to locate the file in my directory.

Well, in theory, I could go even further by opening config.toml and typing the characters into the ignoreFiles field for you automatically:

xfun::rstudio_type('"\\\\.knit\\\\.md$", "\\\\.utf8\\\\.md$"')

but I don't want people to feel the big brother is watching you too closely... 😂

Anyway, I'm very glad that you have found the new check_site() helpful!