-
-
Notifications
You must be signed in to change notification settings - Fork 8
get_code
to return concatenated code string and removed format_expression
#176
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
Changes from all commits
066dd4f
744968b
6aece42
0b4905e
26a27ff
af08aea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,19 +32,24 @@ setGeneric("replace_code", function(object, code) standardGeneric("replace_code" | |
#' @keywords internal | ||
setMethod("replace_code", signature = c("qenv", "character"), function(object, code) { | ||
masked_code <- get_code(object) | ||
masked_code[length(masked_code)] <- code | ||
object@code <- masked_code | ||
code_lines <- unlist(strsplit(masked_code, "\n")) | ||
|
||
if (!is.null(code_lines)) { | ||
code_lines[length(code_lines)] <- code | ||
object@code <- paste(code_lines, collapse = "\n") | ||
} | ||
|
||
object | ||
}) | ||
|
||
#' @keywords internal | ||
setMethod("replace_code", signature = c("qenv", "language"), function(object, code) { | ||
replace_code(object, code = format_expression(code)) | ||
replace_code(object, code = paste(lang2calls(code), collapse = "\n")) | ||
}) | ||
|
||
#' @keywords internal | ||
setMethod("replace_code", signature = c("qenv", "expression"), function(object, code) { | ||
replace_code(object, code = format_expression(code)) | ||
replace_code(object, code = paste(lang2calls(code), collapse = "\n")) | ||
Comment on lines
+47
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm having second thoughts about removing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had a similar thought previously, but following a conversation with @chlebowa it was clarified that the function simply uses paste with a designated collapse parameter after processing with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. We don't want to have too simple wrappers. Check out the use of On top of that, soon we will be down to just a few uses of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. format_expression sounds like a wrapper for paste, where you overwrite default collapse param :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you guys consider having It seems that it is only used this way in the package (other than in tests) ps. and rename it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is recursion in |
||
}) | ||
|
||
#' @keywords internal | ||
|
Uh oh!
There was an error while loading. Please reload this page.