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.

Sign up for GitHub

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

Allow trailing commas in more places #3328

Merged
merged 6 commits into from
Mar 23, 2021
Merged

Allow trailing commas in more places #3328

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
85aa961
Allow trailing commas in more places
hadley Mar 9, 2021
60b8941
Merge branch 'master' into trailing-commas
cpsievert Mar 22, 2021
912d657
Merge branch 'master' into trailing-commas
cpsievert Mar 22, 2021
9305909
update news; no need to rlang::
cpsievert Mar 22, 2021
40013a7
missed one
cpsievert Mar 22, 2021
2ebf373
Update NEWS.md
cpsievert Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ shiny 1.6.0.9000

### New features and improvements

* All uses of `list(...)` have been replaced with `rlang::list2(...)`. By effect, `{rlang}`'s `!!!` operator may now be used to "splice" a list of argument values into `...`. We think this'll be particularly useful for passing a list of `tabPanel()` to their consumers (i.e., `tabsetPanel()`, `navbarPage()`, etc). For example, `tabs <- list(tabPanel("A", "a"), tabPanel("B", "b")); navbarPage(!!!tabs)`. (#3315 and #3328)
Copy link
Member Author

@hadley hadley Mar 22, 2021 •
edited by cpsievert
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* All uses of `list(...)` have been replaced with `rlang::list2(...)`. By effect, `{rlang}`'s `!!!` operator may now be used to "splice" a list of argument values into `...`. We think this'll be particularly useful for passing a list of `tabPanel()` to their consumers (i.e., `tabsetPanel()`, `navbarPage()`, etc). For example, `tabs <- list(tabPanel("A", "a"), tabPanel("B", "b")); navbarPage(!!!tabs)`. (#3315 and #3328)
* All uses of `list(...)` have been replaced with `rlang::list2(...)`. This means that you can use trailing `,` without error and use rlang's `!!!` operator to "splice" a list of argument values into `...`. We think this'll be particularly useful for passing a list of `tabPanel()` to their consumers (i.e., `tabsetPanel()`, `navbarPage()`, etc). For example, `tabs <- list(tabPanel("A", "a"), tabPanel("B", "b")); navbarPage(!!!tabs)`. (#3315 and #3328)

Is that true about splicing though? I thought tagList() already did auto-splicing?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I believe all of htmltools uses dots_list() now. I couldn't find any mention of it in the NEWS though so I figured it wouldn't hurt to say it just generally works now with any ...?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, shouldn't the suggestion be without error instead of with out warning?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops yeah. But I meant doesn't splicing already happen without !!!?

Copy link
Collaborator

@cpsievert cpsievert Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, technically no, but in practice the difference usually isn't important (at least as far as renderTags() is concerned)

> str(tagList(list(tags$a(), tags$div())))
List of 1
 $ :List of 2
  ..$ :List of 3
  .. ..$ name    : chr "a"
  .. ..$ attribs : Named list()
  .. ..$ children: list()
  .. ..- attr(*, "class")= chr "shiny.tag"
  ..$ :List of 3
  .. ..$ name    : chr "div"
  .. ..$ attribs : Named list()
  .. ..$ children: list()
  .. ..- attr(*, "class")= chr "shiny.tag"
 - attr(*, "class")= chr [1:2] "shiny.tag.list" "list"
> str(tagList(!!!list(tags$a(), tags$div())))
List of 2
 $ :List of 3
  ..$ name    : chr "a"
  ..$ attribs : Named list()
  ..$ children: list()
  ..- attr(*, "class")= chr "shiny.tag"
 $ :List of 3
  ..$ name    : chr "div"
  ..$ attribs : Named list()
  ..$ children: list()
  ..- attr(*, "class")= chr "shiny.tag"
 - attr(*, "class")= chr [1:2] "shiny.tag.list" "list"


* Numerous improvements tabset panels (i.e., `tabPanel()`, `navbarMenu()`, `tabsetPanel()`, `navbarPage()`, etc) (#3315):
* Closed #3322: `tabsetPanel()` and `navlistPanel()` gain `header`/`footer` arguments (inspired by `navbarPage()`'s already existing `header`/`footer`), making it easier to include content that should appear on every tab.
* Closed #3313 and #1823: More informative error when non-`tabPanel()`/`shiny.tag` objects are supplied to `...`.
Expand Down
8 changes: 4 additions & 4 deletions R/bootstrap-layout.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ mainPanel <- function(..., width = 8) {
#' }
#' @export
verticalLayout <- function(..., fluid = TRUE) {
lapply(list(...), function(row) {
lapply(list2(...), function(row) {
col <- column(12, row)
if (fluid)
fluidRow(col)
Expand Down Expand Up @@ -433,7 +433,7 @@ verticalLayout <- function(..., fluid = TRUE) {
#' @export
flowLayout <- function(..., cellArgs = list()) {

children <- list(...)
children <- list2(...)
childIdx <- !nzchar(names(children) %||% character(length(children)))
attribs <- children[!childIdx]
children <- children[childIdx]
Expand Down Expand Up @@ -516,7 +516,7 @@ inputPanel <- function(...) {
#' @export
splitLayout <- function(..., cellWidths = NULL, cellArgs = list()) {

children <- list(...)
children <- list2(...)
childIdx <- !nzchar(names(children) %||% character(length(children)))
attribs <- children[!childIdx]
children <- children[childIdx]
Expand Down Expand Up @@ -614,7 +614,7 @@ fillCol <- function(..., flex = 1, width = "100%", height = "100%") {
}

flexfill <- function(..., direction, flex, width = width, height = height) {
children <- list(...)
children <- list2(...)
attrs <- list()

if (!is.null(names(children))) {
Expand Down
2 changes: 1 addition & 1 deletion R/bootstrap.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ bootstrapPage <- function(..., title = NULL, responsive = deprecated(), theme =
tags$head(tags$link(rel="stylesheet", type="text/css", href=theme))
},
# remainder of tags passed to the function
list(...)
list2(...)
)

# If theme is a bslib::bs_theme() object, bootstrapLib() needs to come first
Expand Down
4 changes: 2 additions & 2 deletions R/reactives.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ ReactiveValues <- R6Class(
#' @seealso [isolate()] and [is.reactivevalues()].
#' @export
reactiveValues <- function(...) {
args <- list(...)
args <- list2(...)
if ((length(args) > 0) && (is.null(names(args)) || any(names(args) == "")))
rlang::abort("All arguments passed to reactiveValues() must be named.")

Expand Down Expand Up @@ -1915,7 +1915,7 @@ reactivePoll <- function(intervalMillis, session, checkFunc, valueFunc) {
#' @export
reactiveFileReader <- function(intervalMillis, session, filePath, readFunc, ...) {
filePath <- coerceToFunc(filePath)
extraArgs <- list(...)
extraArgs <- list2(...)

reactivePoll(
intervalMillis, session,
Expand Down
2 changes: 1 addition & 1 deletion R/shiny-options.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ getShinyOption <- function(name, default = NULL) {
#' @aliases shiny-options
#' @export
shinyOptions <- function(...) {
newOpts <- list(...)
newOpts <- list2(...)

if (length(newOpts) > 0) {
# If we're within a session, modify at the session level.
Expand Down
2 changes: 1 addition & 1 deletion R/utils.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,7 +1176,7 @@ reactiveStop <- function(message = "", class = NULL) {
#'
#' }
validate <- function(..., errorClass = character(0)) {
results <- sapply(list(...), function(x) {
results <- sapply(rlang::list2(...), function(x) {
# Detect NULL or NA
if (is.null(x))
return(NA_character_)
Expand Down