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

Add tests to remove_enclosing_curly_braces and use for qenv? #4

Closed
Tracked by #69
nikolas-burkoff opened this issue Mar 30, 2022 · 7 comments · Fixed by #98
Closed
Tracked by #69

Add tests to remove_enclosing_curly_braces and use for qenv? #4

nikolas-burkoff opened this issue Mar 30, 2022 · 7 comments · Fixed by #98

Comments

@nikolas-burkoff
Copy link
Contributor

nikolas-burkoff commented Mar 30, 2022

And simplify - as the function is not nice at all

From https://github.com/insightsengineering/coredev-tasks/issues/147

@donyunardi
Copy link
Contributor

@nikolas-burkoff
Can we close this issue after qenv implementation?

@nikolas-burkoff nikolas-burkoff changed the title Add tests to remove_enclosing_curly_braces Add tests to remove_enclosing_curly_braces and use for qenv? Nov 3, 2022
@nikolas-burkoff
Copy link
Contributor Author

So @donyunardi we can either remove this function entirely OR we can fix it and use it with qenvs to remove the extra "{" and "}" like we used to do with chunks

Good spot @BLAZEWIM

@nikolas-burkoff
Copy link
Contributor Author

ok so this function needs some tlc:

image

so n_spaces_indent is a vector of NA's and 4s and if there's at least one 4 then do the n_rm bit

@nikolas-burkoff
Copy link
Contributor Author

nikolas-burkoff commented Nov 16, 2022

This looks to be a simple replacement to the above - of course the above function may not be what we want...

remove_enclosing_curly_braces_replacement <- function(x) {

  if (length(x) == 0) return(x)

  open_bracket_then_spaces <- "^\\{[[:space:]]*"
  close_bracket_then_spaces <- "^\\}[[:space:]]*"
  blank_line <- "^[[:space:]]*$"
  four_spaces_at_start_of_line <- "^[[:space:]]{4}"

  split_text <- unlist(strsplit(x, "\n", fixed = TRUE))

  # if text begins with "{   \n" and ends with "\n}   "
  if (grepl(open_bracket_then_spaces, utils::head(split_text, 1)) &&
    grepl(close_bracket_then_spaces, utils::tail(split_text, 1))) {

    #remove the first and last line
    split_text <- split_text[-c(1, length(split_text))]

    # if any line is not blank then indent
    if (!all(grepl(blank_line, split_text))){
      return(gsub(four_spaces_at_start_of_line, "", split_text))
    }
  } else {
    return(split_text)
  }
}

@nikolas-burkoff
Copy link
Contributor Author

nikolas-burkoff commented Nov 16, 2022

This task is to

  1. confirm the above function removes the curly brackets and realigns the code - feel free to improve on this/make it work if it doesn't
  2. write tests to show its behaviour
  3. use this function inside get_code() here, here and a few other places in the codebase so that brackets are being removed in SRC including in trace and warnings output
  4. Fix any tests and check with tmg/tmc modules

Example teal app which should then be formatted nicely - this shows the show r code and the trace. Also need to test warnings - see example app e.g. here

image

library(teal.code)
library(teal.widgets)
library(shiny)


ui <- fluidPage(
  verbatim_popup_ui("button", "Press me")
)

server <- function(input, output, session) {
  q <- reactive({
    new_qenv(
      list2env(list(y = 2)), # add x = 1 here to remove the error
      code = quote({
        x <- 1
        y <- 2
      })
    ) |> eval_code(quote({
      if (x == 1) {
        print("x = 1")
      }
    }))
  })


  verbatim_popup_srv("button",
    verbatim_content = reactive(get_code(q())),
    title = "code"
  )
}

shinyApp(ui, server)

@nikolas-burkoff
Copy link
Contributor Author

Added blocked until we get PO input

@nikolas-burkoff
Copy link
Contributor Author

@lcd2yyz is happy for this to go ahead so I've unblocked

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

Successfully merging a pull request may close this issue.

2 participants