Support non-package code #634
Conversation
|
The major downside of this is that it exposes an internal data structure, preventing me from making changes in the future. |
|
Yes, and I would be more than happy to wrap this in some way. However, in a way the structure is already part of the public API because it’s the input to the (public but mostly undocumented) If it helps, this can always be left as a deliberately undocumented type, with the remark that it’s only useful as input to |
R/parse.R
Outdated
| #' Roxygen comment. | ||
| #' @param file_encoding The file encoding. Default: `"UTF-8"`. | ||
| #' @param text Instead of specifying `file`, users can also specify a character | ||
| #' vector `text`, containing the R code. |
There was a problem hiding this comment.
Needs indent. Maybe document with text?
Can you please add @keyword internal
There was a problem hiding this comment.
Did you mean “… document with file”? Otherwise I’m not sure what you mean by this.
R/parse.R
Outdated
| } | ||
|
|
||
| options <- list(wrap = wrap, markdown = markdown) | ||
| parse_blocks(file, env, registry = registry, global_options = options, |
There was a problem hiding this comment.
Can you please follow new standard for wrapping long lines? (http://style.tidyverse.org/syntax.html#long-lines)
There was a problem hiding this comment.
Done, here. Does this also apply to the parameter list in the function definition?
R/parse.R
Outdated
| parse_code <- function(file, env, registry = default_tags(), wrap = FALSE, | ||
| markdown = markdown_global_default, | ||
| file_encoding = "UTF-8", text) { | ||
| if (missing(file) && ! missing(text)) { |
There was a problem hiding this comment.
I don't love interfaces where you have to specify exactly one of a set of arguments. I don't see any alternative here, but I think you need to make the structure clearer by placing file and text close together in the argument list. It might be better to default text = NULL and then condition on that.
* Mark function as “internal” * Move `text` parameter to be adjacent to `file`, document together * Make `text` parameter default to `NULL` * Follow tidyverse style for wrapping long lines
|
Please take another look. |
R/parse.R
Outdated
| if (missing(file) && ! missing(text)) { | ||
| #' @keywords internal | ||
| parse_code <- function( | ||
| file, text = NULL, |
There was a problem hiding this comment.
Oh except the rule for function blocks is that you indent with the ( in function. I need to document that in style.
There was a problem hiding this comment.
Ah yes, I found an example elsewhere in the code base. Makes sense.
|
Thanks! |
2 participants
As suggested by @hadley in #273, here’s a suggested API for trivially adding single-file parsing support for Roxygen2.
At the moment the proposed change is pretty minimal (but already sufficient for my purposes): it adds a single, exported function
parse_codethat takes a file or a piece of R code as text, along with an environment of parsed R expressions, and parses the Roxygen tags associated with these objects, to be returned to the user.I could alternatively/additionally imagine providing a
roxigenize_filefunction (as shown in branchsingle-file-support-roxygenize).I would greatly appreciate feedback to make this PR suitable for merging into Roxygen2.
Here’s why I think it’s useful:
I appreciate that “this is currently outside the scope of roxygen” as Hadley said. However, I think it’s a worthwhile addition because:
tools::parse_Rdand then to e.g.tools::Rd2txt, and displayed. In fact, that’s what ‹modules› is doing. If there’s demand, it would be trivial to refactor the “display help from Rd” functionality into a separate package.