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

improve format_expression #159

Open
chlebowa opened this issue Oct 31, 2023 · 4 comments
Open

improve format_expression #159

chlebowa opened this issue Oct 31, 2023 · 4 comments

Comments

@chlebowa
Copy link
Contributor

chlebowa commented Oct 31, 2023

I think we should improve format_expression a little.

Consider:

ll1 <- list(quote(i <- iris), quote(m <- mtcars))
ll2 <- list(quote({i <- iris; m <- mtcars}))
char1 <- c("i <- iris", "m <- mtcars")
char2 <- "
  i <- iris
  m <- mtcars
"

Currently

format_expression <- function(code) {
  code <- lang2calls(code)
  paste(code, collapse = "\n")
}

> format_expression(ll1)
[1] "i <- iris\nm <- mtcars" 
> format_expression(ll2)
[1] "i <- iris\nm <- mtcars"
> format_expression(char1)
[1] "c(\"i <- iris\", \"m <- mtcars\")"
> format_expression(char2)
[1] "\n  i <- iris\n  m <- mtcars\n"

unlist separates character vector

> format_expression <- function(code) {
+   code <- lang2calls(code)
+   paste(unlist(code), collapse = "\n")
+ }

> format_expression(ll1)
[1] "i <- iris\nm <- mtcars"
> format_expression(ll2)
[1] "i <- iris\nm <- mtcars"
> format_expression(char1)
[1] "i <- iris\nm <- mtcars"
> format_expression(char2)
[1] "\n  i <- iris\n  m <- mtcars\n"

trimws removes some excess white space

format_expression <- function(code) {
  code <- lang2calls(code)
  trimws(paste(unlist(code), collapse = "\n"))
}

> format_expression(ll1)
[1] "i <- iris\nm <- mtcars"
> format_expression(ll2)
[1] "i <- iris\nm <- mtcars"
> format_expression(char1)
[1] "i <- iris\nm <- mtcars"
> format_expression(char2)
[1] "i <- iris\n  m <- mtcars"

Question is: do we really need it? This is an internal function and I'm not sure if there are constrains on the input. Then again we may want to use it in another place (there's a PR in teal.data that does).

@m7pr
Copy link
Contributor

m7pr commented Dec 18, 2023

@chlebowa is this still relevant? There is no more format_expression in this package and not in teal.data

@chlebowa
Copy link
Contributor Author

I guess if the answer to "do we need it" is yes, then we should give it some thought. Who knows, maybe even bring back format_expression?

@averissimo
Copy link
Contributor

trimws might mess up some indentation. {styler} might be a good way of getting rid of any formatting issues, but would involve an extra-dependency

format_expression <- function(code) {
  paste(unlist(teal.code:::lang2calls(code)), collapse = "\n")
}

format_expression_trimws <- function(code) {
  trimws(paste(unlist(teal.code:::lang2calls(code)), collapse = "\n"))
}

format_expression_styler <- function(code) {
  paste(styler::style_text(teal.code:::lang2calls(code)), collapse = "\n")
}

char4 <- "
  while (FALSE) {
    i <- iris
    m <- mtcars
  }
"

format_expression(char4)
#> [1] "\n  while (FALSE) {\n    i <- iris\n    m <- mtcars\n  }\n"
format_expression_trimws(char4)
#> [1] "while (FALSE) {\n    i <- iris\n    m <- mtcars\n  }"
format_expression_styler(char4)
#> [1] "while (FALSE) {\n  i <- iris\n  m <- mtcars\n}"

ps. ignore this if there is a downstream formatting I'm missing

@chlebowa
Copy link
Contributor Author

ps. ignore this if there is a downstream formatting I'm missing

There is, actually:

> teal.widgets::verbatim_popup_srv
function (id, verbatim_content, title, style = FALSE, disabled = shiny::reactiveVal(FALSE)) 
{
    checkmate::assert_string(id)
    checkmate::assert_string(title)
    checkmate::assert_flag(style)
    checkmate::assert_class(disabled, classes = "reactive")
    moduleServer(id, function(input, output, session) {
        ns <- session$ns
        modal_content <- format_content(verbatim_content, style)
        button_click_observer(click_event = shiny::reactive(input$button), 
            copy_button_id = ns("copy_button"), copied_area_id = ns("verbatim_content"), 
            modal_title = title, modal_content = modal_content, 
            disabled = disabled)
    })
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants