roxygen2 icon indicating copy to clipboard operation
roxygen2 copied to clipboard

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

Open hadley opened this issue 3 years ago • 4 comments

To match other packages

hadley avatar Apr 09 '22 12:04 hadley

From Roxygen also, I assume?

gaborcsardi avatar Apr 09 '22 12:04 gaborcsardi

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 https://github.com/r-lib/roxygen2/issues/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)

hadley avatar Apr 18 '22 13:04 hadley

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?

gaborcsardi avatar Apr 18 '22 14:04 gaborcsardi

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?

hadley avatar Apr 18 '22 14:04 hadley