insightsengineering / teal.code

Code storage and execution class for teal applications
https://insightsengineering.github.io/teal.code/
Other
11 stars 7 forks source link

improve `format_expression` #159

Open chlebowa opened 10 months ago

chlebowa commented 10 months ago

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 commented 9 months ago

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

chlebowa commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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)
    })
}