-
Notifications
You must be signed in to change notification settings - Fork 24
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
function for making new blog posts #171
base: main
Are you sure you want to change the base?
Conversation
yml_values <- yml_values[names(yml_values) != "categories"] | ||
} | ||
yml_values <- yaml::as.yaml(yml_values) | ||
yml_values <- paste0("---\n", yml_values, "---\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, maybe there is a better API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While dealing with YAML in quarto context we have the issue of yaml R package being 1.1 spec, and Quarto using 1.2 spec.
So it requires some specific handlers as workaround. See
Lines 6 to 16 in d71b135
write_yaml <- function(x, file) { | |
handlers <- list( | |
# Handle yes/no from 1.1 to 1.2 | |
# https://github.com/vubiostat/r-yaml/issues/131 | |
logical = function(x) { | |
value <- ifelse(x, "true", "false") | |
structure(value, class = "verbatim") | |
} | |
) | |
yaml::write_yaml(x, file, handlers = handlers) | |
} |
We probably need to adapt for generating not in a file, or refactor the handlers so that we can use them in other functions.
Regaring ---
, I believe this is good enough.
This post creation may also be done through templating 🤔 Having a whisker template (or else) to file out with some information. Probably more complex than necessary, but we did that for bookdown skeleton for example using placeholder in template file (https://github.com/rstudio/bookdown/blob/f244cf12bf2c2d7106ac6322b2b2a5796d4ef0c8/R/skeleton.R#L66-L83)
I am fine with current way
GHA failures are now down to infrastructure issues, not failing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot ! This will be really useful.
Do you think this will be used more interactively or not ? Just curious about your use case.
Just a few suggestions below. I see that we may need to really start using fs
Also, I am wondering if we should have more check that this function is used in the right place.
- website project where blog is supported not another type or no project
- called in the root of the project so that posts folder is created in the right place
- or no matter where called, the new post is created in the right place from the root project
Again, thanks !!
DESCRIPTION
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As those two deps are only required for the new blog post function, and this function is aimed to be used mostly interactively, I wonder if we could rely on rlang::check_installed()
By using if(rlang::check_installed("whoami"))
, user will be asked to install if missing when used interactively.
This would allow to make them suggested dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For fs, we probably could start using it in this package.
@@ -1,5 +1,7 @@ | |||
# quarto (development version) | |||
|
|||
- Added a `new_blog_post()` function (#22). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Added a `new_blog_post()` function (#22). | |
- Added a `new_blog_post()` function (thanks, @topepo , #22). |
#' default, the title is adapted as the directory name. | ||
#' @param open A logical: have the default editor open a window to edit the | ||
#' `index.qmd` file? | ||
#' @param call A call object for reporting errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that the prefered way is to use
@inheritParams rlang::args_error_context
From https://rlang.r-lib.org/reference/args_error_context.html and usage at several tidyverse packages
yml_values <- yml_values[names(yml_values) != "categories"] | ||
} | ||
yml_values <- yaml::as.yaml(yml_values) | ||
yml_values <- paste0("---\n", yml_values, "---\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While dealing with YAML in quarto context we have the issue of yaml R package being 1.1 spec, and Quarto using 1.2 spec.
So it requires some specific handlers as workaround. See
Lines 6 to 16 in d71b135
write_yaml <- function(x, file) { | |
handlers <- list( | |
# Handle yes/no from 1.1 to 1.2 | |
# https://github.com/vubiostat/r-yaml/issues/131 | |
logical = function(x) { | |
value <- ifelse(x, "true", "false") | |
structure(value, class = "verbatim") | |
} | |
) | |
yaml::write_yaml(x, file, handlers = handlers) | |
} |
We probably need to adapt for generating not in a file, or refactor the handlers so that we can use them in other functions.
Regaring ---
, I believe this is good enough.
This post creation may also be done through templating 🤔 Having a whisker template (or else) to file out with some information. Probably more complex than necessary, but we did that for bookdown skeleton for example using placeholder in template file (https://github.com/rstudio/bookdown/blob/f244cf12bf2c2d7106ac6322b2b2a5796d4ef0c8/R/skeleton.R#L66-L83)
I am fine with current way
make_post_yaml <- function(title, ...) { | ||
default_values <- list( | ||
title = tools::toTitleCase(title), | ||
author = tools::toTitleCase(whoami::fullname("Your name")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could document for .Rd file that whoami is used as default when available
We could also easily make this deps a suggest by using it only if available
tools::toTitleCase(if(rlang::is_installed("whoami")) whoami::fullname("Your name") else "Your name")
temp_dir <- withr::local_tempdir() | ||
dir_path <- fs::path(temp_dir, "test-blog-project") | ||
|
||
withr::defer(fs::dir_delete(dir_path), envir = rlang::current_env()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
withr::local_tempdir()
already include a defer
call to unlink()
recursively its content. So I believe no need for a new defer
call
setwd(dir_path) | ||
withr::defer(setwd(current_dir), envir = rlang::current_env()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually I use withr::local_dir()
to change working directory for the rest of the test
This should be enough
withr::local_dir(fs::path(temp_dir, "test-blog-project"))
setwd(dir_path) | ||
withr::defer(setwd(current_dir), envir = rlang::current_env()) | ||
|
||
Sys.setenv(FULLNAME="Max Kuhn") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should keep using withr for that
Sys.setenv(FULLNAME="Max Kuhn") | |
withr::local_envvar(list(FULLNAME = "Max Kuhn")) |
I have updated main to solve this. It should be ok now. |
For #22
Adds some dependencies (fs, whoami).