From 233c2f312137495392450ee55090ab81d0bd0b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Thu, 19 Dec 2024 12:27:01 +0000 Subject: [PATCH] Uses `{cli}` instead of `{crayon}` (#1433) # Pull Request Fixes #1417 #### Changes description - Replace styling in `format.module(s)` with `cli` - Remove `strip_style` function in favor of `cli::ansi_strip` - Reformat argument style in line with rest of file/package - Forces text color to black on white background (relevant in dark mode) - Align `transformators` in module print #### Visual changes (minor corrections): ![image](https://github.com/user-attachments/assets/76e770f3-6061-448d-bbd5-baf4b740a633) ![image](https://github.com/user-attachments/assets/659e2c8d-b8ba-41f2-8c1b-a9d39762a57e) --- .pre-commit-config.yaml | 2 +- DESCRIPTION | 4 ++-- R/module_teal_data.R | 2 +- R/modules.R | 32 +++++++++++++++++--------------- R/utils.R | 14 -------------- tests/testthat/test-modules.R | 8 ++++---- 6 files changed, 25 insertions(+), 37 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 646a00dbf2..cdab3ba284 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,7 +19,7 @@ repos: - davidgohel/gdtools # for flextable - mirai - checkmate - - crayon + - cli - jsonlite - lifecycle - logger diff --git a/DESCRIPTION b/DESCRIPTION index da42907547..497adc7c9b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -41,7 +41,7 @@ Depends: teal.slice (>= 0.5.1.9015) Imports: checkmate (>= 2.1.0), - crayon, + cli, jsonlite, lifecycle (>= 0.2.0), logger (>= 0.2.0), @@ -77,7 +77,7 @@ RdMacros: lifecycle Config/Needs/verdepcheck: rstudio/shiny, insightsengineering/teal.data, insightsengineering/teal.slice, mllg/checkmate, jeroen/jsonlite, - r-lib/lifecycle, daroczig/logger, shikokuchuo/mirai, r-lib/crayon, + r-lib/lifecycle, daroczig/logger, shikokuchuo/mirai, r-lib/cli, shikokuchuo/nanonext, rstudio/renv, r-lib/rlang, daattali/shinyjs, insightsengineering/teal.code, insightsengineering/teal.logger, insightsengineering/teal.reporter, insightsengineering/teal.widgets, diff --git a/R/module_teal_data.R b/R/module_teal_data.R index 8619b7cedd..a177f6d9de 100644 --- a/R/module_teal_data.R +++ b/R/module_teal_data.R @@ -169,7 +169,7 @@ srv_validate_error <- function(id, data, validate_shiny_silent_error) { FALSE, paste( "Error when executing the `data` module:", - strip_style(paste(data()$message, collapse = "\n")), + cli::ansi_strip(paste(data()$message, collapse = "\n")), "\nCheck your inputs or contact app developer if error persists.", collapse = "\n" ) diff --git a/R/modules.R b/R/modules.R index 2b27d5bb41..f67e16a14d 100644 --- a/R/modules.R +++ b/R/modules.R @@ -339,9 +339,11 @@ modules <- function(..., label = "root") { #' ) #' cat(format(mod)) #' @export -format.teal_module <- function( - x, is_last = FALSE, parent_prefix = "", - what = c("datasets", "properties", "ui_args", "server_args", "transformators"), ...) { +format.teal_module <- function(x, + is_last = FALSE, + parent_prefix = "", + what = c("datasets", "properties", "ui_args", "server_args", "transformators"), + ...) { empty_text <- "" branch <- if (is_last) "L-" else "|-" current_prefix <- paste0(parent_prefix, branch, " ") @@ -353,7 +355,7 @@ format.teal_module <- function( } else { colon_space <- paste(rep(" ", label_width), collapse = "") - first_item <- sprintf("%s (%s)", names(lst)[1], crayon::silver(class(lst[[1]])[1])) + first_item <- sprintf("%s (%s)", names(lst)[1], cli::col_silver(class(lst[[1]])[1])) rest_items <- if (length(lst) > 1) { paste( vapply( @@ -363,7 +365,7 @@ format.teal_module <- function( "%s%s (%s)", paste0(content_prefix, "| ", colon_space), name, - crayon::silver(class(lst[[name]])[1]) + cli::col_silver(class(lst[[name]])[1]) ) }, character(1) @@ -384,40 +386,40 @@ format.teal_module <- function( empty_text } - output <- pasten(current_prefix, crayon::bgWhite(x$label)) + output <- pasten(current_prefix, cli::bg_white(cli::col_black(x$label))) if ("datasets" %in% what) { output <- paste0( output, - content_prefix, "|- ", crayon::yellow("Datasets : "), paste(x$datanames, collapse = ", "), "\n" + content_prefix, "|- ", cli::col_yellow("Datasets : "), paste(x$datanames, collapse = ", "), "\n" ) } if ("properties" %in% what) { output <- paste0( output, - content_prefix, "|- ", crayon::blue("Properties:"), "\n", - content_prefix, "| |- ", crayon::cyan("Bookmarkable : "), bookmarkable, "\n", - content_prefix, "| L- ", crayon::cyan("Reportable : "), reportable, "\n" + content_prefix, "|- ", cli::col_blue("Properties:"), "\n", + content_prefix, "| |- ", cli::col_cyan("Bookmarkable : "), bookmarkable, "\n", + content_prefix, "| L- ", cli::col_cyan("Reportable : "), reportable, "\n" ) } if ("ui_args" %in% what) { ui_args_formatted <- format_list(x$ui_args, label_width = 19) output <- paste0( output, - content_prefix, "|- ", crayon::green("UI Arguments : "), ui_args_formatted, "\n" + content_prefix, "|- ", cli::col_green("UI Arguments : "), ui_args_formatted, "\n" ) } if ("server_args" %in% what) { server_args_formatted <- format_list(x$server_args, label_width = 19) output <- paste0( output, - content_prefix, "|- ", crayon::green("Server Arguments : "), server_args_formatted, "\n" + content_prefix, "|- ", cli::col_green("Server Arguments : "), server_args_formatted, "\n" ) } if ("transformators" %in% what) { output <- paste0( output, - content_prefix, "L- ", crayon::magenta("Transformators : "), transformators, "\n" + content_prefix, "L- ", cli::col_magenta("Transformators : "), transformators, "\n" ) } @@ -526,12 +528,12 @@ format.teal_module <- function( #' @export format.teal_modules <- function(x, is_root = TRUE, is_last = FALSE, parent_prefix = "", ...) { if (is_root) { - header <- pasten(crayon::bold("TEAL ROOT")) + header <- pasten(cli::style_bold("TEAL ROOT")) new_parent_prefix <- " " #' Initial indent for root level } else { if (!is.null(x$label)) { branch <- if (is_last) "L-" else "|-" - header <- pasten(parent_prefix, branch, " ", crayon::bold(x$label)) + header <- pasten(parent_prefix, branch, " ", cli::style_bold(x$label)) new_parent_prefix <- paste0(parent_prefix, if (is_last) " " else "| ") } else { header <- "" diff --git a/R/utils.R b/R/utils.R index 5bd81b7a28..46fcddc1a3 100644 --- a/R/utils.R +++ b/R/utils.R @@ -379,20 +379,6 @@ get_unique_labels <- function(labels) { make.unique(gsub("[^[:alnum:]]", "_", tolower(labels)), sep = "_") } -#' Remove ANSI escape sequences from a string -#' @noRd -strip_style <- function(string) { - checkmate::assert_string(string) - - gsub( - "(?:(?:\\x{001b}\\[)|\\x{009b})(?:(?:[0-9]{1,3})?(?:(?:;[0-9]{0,3})*)?[A-M|f-m])|\\x{001b}[A-M]", - "", - string, - perl = TRUE, - useBytes = TRUE - ) -} - #' @keywords internal #' @noRd pasten <- function(...) paste0(..., "\n") diff --git a/tests/testthat/test-modules.R b/tests/testthat/test-modules.R index 5a1da7b37f..943de3d55d 100644 --- a/tests/testthat/test-modules.R +++ b/tests/testthat/test-modules.R @@ -510,7 +510,7 @@ testthat::test_that("format.teal_modules returns proper structure", { appended_mods <- append_module(mods, mod3) testthat::expect_setequal( - strsplit(gsub("\033\\[[0-9;]*m", "", format(appended_mods)), "\n")[[1]], + strsplit(cli::ansi_strip(format(appended_mods)), "\n")[[1]], c( "TEAL ROOT", " |- a", @@ -520,7 +520,7 @@ testthat::test_that("format.teal_modules returns proper structure", { " | | L- Reportable : FALSE", " | |- UI Arguments : ", " | |- Server Arguments : ", - " | L- Transformators : ", + " | L- Transformators : ", " |- c", " | |- Datasets : all", " | |- Properties:", @@ -528,7 +528,7 @@ testthat::test_that("format.teal_modules returns proper structure", { " | | L- Reportable : FALSE", " | |- UI Arguments : ", " | |- Server Arguments : ", - " | L- Transformators : ", + " | L- Transformators : ", " L- c", " |- Datasets : all", " |- Properties:", @@ -536,7 +536,7 @@ testthat::test_that("format.teal_modules returns proper structure", { " | L- Reportable : FALSE", " |- UI Arguments : ", " |- Server Arguments : ", - " L- Transformators : " + " L- Transformators : " ) ) })