From 39b58691bb113df4f9f8682bc86f1c77e97967f6 Mon Sep 17 00:00:00 2001 From: Mauricio Vargas Sepulveda Date: Mon, 25 Aug 2025 19:11:59 -0400 Subject: [PATCH 1/2] implement non-interactive flow for create_if_needed() --- R/add_r_files.R | 3 +- R/modules_fn.R | 3 +- R/utils.R | 99 ++++++++++++++++++++++++++++--------------------- 3 files changed, 61 insertions(+), 44 deletions(-) diff --git a/R/add_r_files.R b/R/add_r_files.R index 29fa5108..f133d7be 100644 --- a/R/add_r_files.R +++ b/R/add_r_files.R @@ -39,7 +39,8 @@ add_r_files <- function( dir_created <- create_if_needed( "R", - type = "directory" + type = "directory", + warn_if_exists = FALSE ) if (!dir_created) { diff --git a/R/modules_fn.R b/R/modules_fn.R index 1fb5c5b4..66075c84 100644 --- a/R/modules_fn.R +++ b/R/modules_fn.R @@ -85,7 +85,8 @@ add_module <- function( golem_wd, "R" ), - type = "directory" + type = "directory", + warn_if_exists = FALSE ) if (!dir_created) { diff --git a/R/utils.R b/R/utils.R index baadf0b9..30299193 100644 --- a/R/utils.R +++ b/R/utils.R @@ -17,7 +17,9 @@ create_if_needed <- function( "file", "directory" ), - content = NULL + content = NULL, + overwrite = FALSE, + warn_if_exists = TRUE ) { type <- match.arg( type @@ -25,65 +27,78 @@ create_if_needed <- function( # Check if file or dir already exist if (type == "file") { - dont_exist <- Negate( - fs_file_exists - )( - path - ) + already_exists <- fs_file_exists(path) } else if (type == "directory") { - dont_exist <- Negate( - fs_dir_exists - )( - path - ) + already_exists <- fs_dir_exists(path) + } + + # If it already exists, handle overwrite logic + if (already_exists) { + if (!overwrite) { + if (rlang_is_interactive()) { + # In interactive mode, ask user if they want to overwrite + ask <- yesno( + sprintf( + "The %s %s already exists, overwrite?", + basename(path), + type + ) + ) + if (!ask) { + return(TRUE) # File exists, user doesn't want to overwrite + } + } else { + # In non-interactive mode, warn but don't overwrite (if warnings enabled) + if (warn_if_exists) { + warning( + sprintf( + "The %s %s already exists and will not be overwritten. Set overwrite = TRUE to force overwrite.", + basename(path), + type + ), + call. = FALSE + ) + } + return(TRUE) # File exists, not overwriting + } + } + # If we reach here, either overwrite = TRUE or user said yes to overwrite } - # If it doesn't exist, ask if we are allowed to create it - if (dont_exist) { - if (rlang_is_interactive()) { + + # If it doesn't exist, or we're overwriting, create it + if (!already_exists || overwrite) { + if (rlang_is_interactive() && !already_exists) { + # In interactive mode, ask if we should create new files/dirs ask <- ask_golem_creation_file( path, type ) # Return early if the user doesn't allow if (!ask) { - return( - FALSE - ) + return(FALSE) } - # Create the file - if (type == "file") { - fs_file_create( - path - ) + } + + # Create the file or directory + if (type == "file") { + fs_file_create(path) + if (!is.null(content)) { write( content, path, - append = TRUE - ) - } else if (type == "directory") { - fs_dir_create( - path, - recurse = TRUE + append = !overwrite # If overwriting, don't append ) } - } else { - # We don't create the file if we are not in - # interactive mode - stop( - sprintf( - "The %s %s doesn't exist.", - basename( - path - ), - type - ) + } else if (type == "directory") { + fs_dir_create( + path, + recurse = TRUE ) } } + # TRUE means that file exists (either created or already there) - return( - TRUE - ) + return(TRUE) } ask_golem_creation_file <- function( From 6f5dc9c64b561543712ab7251c7de1ae81670db1 Mon Sep 17 00:00:00 2001 From: Mauricio Vargas Sepulveda Date: Mon, 25 Aug 2025 20:44:44 -0400 Subject: [PATCH 2/2] ad 1207 here so it passes the tests --- R/utils.R | 36 +++++++++++++++++++++ tests/testthat/test-utils.R | 64 +++++++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/R/utils.R b/R/utils.R index 30299193..56ef6d61 100644 --- a/R/utils.R +++ b/R/utils.R @@ -430,3 +430,39 @@ signal_arg_is_deprecated <- function( ) } } + +#' Sanitize R names +#' +#' Convert a string to a valid R name for functions and variables +#' +#' @param name character string to sanitize +#' @return character string with a valid R name +#' @noRd +sanitize_r_name <- function(name) { + if (is.na(name) || nchar(name) == 0) { + return("unnamed") + } + + # Convert to lowercase and replace spaces with underscores + name <- tolower(name) + name <- gsub("[[:space:]]+", "_", name) + + # Replace any non-alphanumeric characters with underscores + name <- gsub("[^a-zA-Z0-9_]", "_", name) + + # Remove leading/trailing underscores and consolidate multiple underscores + name <- gsub("^_+|_+$", "", name) + name <- gsub("_+", "_", name) + + # If name starts with a number, prefix with 'x' + if (grepl("^[0-9]", name)) { + name <- paste0("x", name) + } + + # If name is empty after cleaning, use "unnamed" + if (nchar(name) == 0) { + name <- "unnamed" + } + + return(name) +} diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 371ac9fc..7ad5b843 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -15,7 +15,9 @@ test_that("rlang_is_interactive() works", { }) test_that("create_if_needed creates a file if required", { - expect_error( + # Test: Non-interactive mode should now succeed and create the file + temp_file <- tempfile() + expect_true( testthat::with_mocked_bindings( rlang_is_interactive = function() { return( @@ -24,11 +26,13 @@ test_that("create_if_needed creates a file if required", { }, code = { create_if_needed( - tempfile() + temp_file ) } ) ) + expect_true(file.exists(temp_file)) + unlink(temp_file) expect_false( testthat::with_mocked_bindings( rlang_is_interactive = function() { @@ -489,3 +493,59 @@ test_that("signal_path_is_deprecated works", { regexp = "mons" ) }) + +test_that("sanitize_r_name works correctly", { + # Test spaces to underscores + expect_equal( + golem:::sanitize_r_name("ma fonction"), + "ma_fonction" + ) + + # Test special characters + expect_equal( + golem:::sanitize_r_name("my-special@function!"), + "my_special_function" + ) + + # Test uppercase to lowercase + expect_equal( + golem:::sanitize_r_name("UPPERCASE Function Name"), + "uppercase_function_name" + ) + + # Test leading number + expect_equal( + golem:::sanitize_r_name("123test"), + "x123test" + ) + + # Test multiple underscores consolidation + expect_equal( + golem:::sanitize_r_name("test___multiple___underscores"), + "test_multiple_underscores" + ) + + # Test leading/trailing underscores removal + expect_equal( + golem:::sanitize_r_name("_test_function_"), + "test_function" + ) + + # Test empty name + expect_equal( + golem:::sanitize_r_name(""), + "unnamed" + ) + + # Test NA name + expect_equal( + golem:::sanitize_r_name(NA_character_), + "unnamed" + ) + + # Test already valid name + expect_equal( + golem:::sanitize_r_name("valid_name"), + "valid_name" + ) +})