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

Plan transition from RoxygenNote to Config/Roxygen/... #1328

Open
hadley opened this issue Apr 9, 2022 · 4 comments
Open

Plan transition from RoxygenNote to Config/Roxygen/... #1328

hadley opened this issue Apr 9, 2022 · 4 comments
Labels
feature a feature request or enhancement

Comments

@hadley
Copy link
Member

hadley commented Apr 9, 2022

To match other packages

@gaborcsardi
Copy link
Member

From Roxygen also, I assume?

@hadley hadley added the feature a feature request or enhancement label Apr 18, 2022
@hadley
Copy link
Member Author

hadley commented Apr 18, 2022

e.g.

Config/roxygen2/markdown: TRUE
Config/roxygen2/load: installed
Config/roxygen2/version: 7.1.2.9000

This needs to affect load_options() (probably adding a new load_options_config()) and update_roxygen_version(). Maybe we can always remove RoygenNote?

@gaborcsardi how do you think the values should be parsed? Read a string and then call type.convert()? Or parse and then evaluate? I guess roclets needs to be a vector, so it's better to be consistent and parse and eval?

Config/roxygen2/roclets: c("rd", "namespace")
Config/roxygen2/markdown: TRUE
Config/roxygen2/load: "installed"
Config/roxygen2/version: "7.1.2.9000"

But having to always wrap everything in quotes is annoying, so maybe better to parse roclets with comma delimiter?

Config/roxygen2/roclets: rd, namespace
Config/roxygen2/markdown: TRUE
Config/roxygen2/load: installed
Config/roxygen2/version: 7.1.2.9000

When we do this we should probably also work to get the IDE to respect these options, instead of maintaining it's own list in the .Rproj.

Also need to consider #1202, i.e. how to provide alternative to the current hack where we rely on the Roxygen field being evaluated (e.g. https://github.com/tidyverse/dtplyr/blob/main/DESCRIPTION#L44)

@gaborcsardi
Copy link
Member

I agree that having to specify a valid R expression is a pain in the neck. It does give us a lot of flexibility, but it is doubtful that we would ever need this flexibility in practice.

OTOH it would be great to have some principled DSL for the values. A somewhat weird idea, would it make sense to make it YAML? Then we would write:

Config/roxygen2/roclets: 
    - rd
    - namespace
Config/roxygen2/markdown: true
Config/roxygen2/load: installed
Config/roxygen2/version: 7.1.2.9000

and the format would be well defined and extensible, and parsing would be easy:

yaml::yaml.load("Config/roxygen2/roclets:
+     - rd
+     - namespace
+ Config/roxygen2/markdown: true
+ Config/roxygen2/load: installed
+ Config/roxygen2/version: 7.1.2.9000
+ ")
$`Config/roxygen2/roclets`
[1] "rd"        "namespace"

$`Config/roxygen2/markdown`
[1] TRUE

$`Config/roxygen2/load`
[1] "installed"

$`Config/roxygen2/version`
[1] "7.1.2.9000"

YAML would also give us a principled way to evaluate R expressions, should we need that.

I don't deny that it is weird to embed YAML into DCF, and there are possibly interactions between the two formats that could make one of the parsers fail?

@hadley
Copy link
Member Author

hadley commented Apr 18, 2022

I wondered about yaml syntax too, but it does seem rather big for this simple problem. Maybe type.convert(scan(text, "character", sep = ",", strip.white = TRUE)) is enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants