[Bug]: Snapshot names collide

[averissimo]

What happened?

How to reproduce:

• Open any of the teal.gallery applications (https://genentech.shinyapps.io/nest_longitudinal_main/)
• Switch to tab that has Filter Panel
• Create new snapshot named Empty Snapshot (space as separator between words)
• Create new snapshot named Empty.Snapshot (. as separator between words)

This is due to the base::make.names function generating syntathically valid names for use in R.

Additionally, shinytest2::get_download() function does not work well with output ID names that have dots in them.

Suggestion:

• Use a custom function that creates unique names based on the initial string and replace the usage in {teal}

#' Make Syntactically Valid Names
#'
#' Extends [base::make.names()] to allow to use underscore as a separator and
#' to add a hash to the end of the name.
#'
#' @param names (`character`) vector of names coerced to syntactically valid names.
#' This is coerced to character if necessary
#' @inherit base::make.names return
#' @keywords internal
#'
make_names <- function(names) {
  checkmate::assert_character(names)
  names <- trimws(names)
  sprintf(
    "%s-%s",
    gsub("[.]", "_", make.names(names)),
    substr(vapply(names, rlang::hash, "character"), 0L, 5L)
  )
}

[chlebowa]

This is a feature, not a bug. The user is encouraged to pick better names. They can add a digit themselves.

[averissimo]

I don't think this is robust enough to filter out bad names. A Snapshot, A.Snapshot and A-Snapshot collide while A_Snapshot and a few other exceptions don't

We could debate if having symbols produce bad names, but I'm ok with the rationale. ~However, whitespace seems problematic as it wouldn't be visible on the UI.~

make.names(c("A Snapshot", "A.Snapshot", "A-Snapshot", "A$Snapshot", "A/Snapshot", "A%Snapshot"))
#> [1] "A.Snapshot" "A.Snapshot" "A.Snapshot" "A.Snapshot" "A.Snapshot"
#> [6] "A.Snapshot"
make.names("prefix\\|!\"#$%&/()=?»«'*+`´ºª~^çÇ.-_,;:<>¨·€ß@£§½¬{[]}¸¹¡suffix")
#> [1] "prefix....................ºª..çÇ.._........ß............suffix"
make.names(c("A_Snapshot", " A Snapshot ", "   A Snapshot   "))
#> [1] "A_Snapshot"        "X.A.Snapshot."     "X...A.Snapshot..."

~Maybe we could extend this feature to filter out whitespace and ALL the symbols (trimws(gsub("\\W|_", ".", trimws(names)), whitespace = "[.]")) :smile:~

make_names <- function(names) {
  checkmate::assert_character(names)
  trimws(gsub("\\W|_", ".", make.names(trimws(names))), whitespace = "[.]")}
}

[averissimo]

Nevermind, whitespace is protected somewhere before make.names call.

Allowing only "_" and a few exceptions seems reasonable

[chlebowa]

Leading and trailing white space is removed, yes.