-
Notifications
You must be signed in to change notification settings - Fork 0
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
Release 0.1.0 #13
Release 0.1.0 #13
Conversation
R/box_lsp.R
Outdated
#' | ||
#' Custom \{languageserver\} parser hook for \{box\} modules. | ||
#' | ||
#' @param expr An R expression to evaluate | ||
#' @param action A list of action functions from `languageserver:::parse_expr()`. | ||
#' @returns Used for side-effects provided by the `action` list of functions. | ||
#' | ||
#' @examples | ||
#' \dontrun{ | ||
#' box_use_parser(expr = expression(box::use(fs)), action = new.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.
The action
argument does not exactly expect an environment. action
is a list of functions:
assign(symbol, value, type = get_expr_type(value))
update(...)
parse(expr)
parse_args(args)
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.
That was my wild guess based on this: https://github.com/REditorSupport/languageserver/blob/master/R/document.R#L317
We need an example here, what do you suggest?
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.
Well... this is what I use when working on it:
action <- list(
assign = function(symbol, value) {
cat(paste("ASSIGN: ", symbol, value, "\n"))
},
update = function(packages) {
cat(paste("Packages: ", packages, "\n"))
},
parse = function(x) {
cat(paste("Parse: ", names(x), x, "\n"))
},
parse_args = function(x) {
cat(paste("Parse Args: ", names(x), x, "\n"))
}
)
Same as in the readme.
Are we sure that it is safe to set the repo to public? I checked, I think it is. |
I think it should be fine. |
Ok. Will set to public now. |
Might also be good to update the version numbers in the README table to 0.1.0 |
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.
LGTM
Pre-release cleanup.