From 49fb23a917682e9f167c7158f9bd0527666d50e9 Mon Sep 17 00:00:00 2001
From: Davis Vaughan <davis@rstudio.com>
Date: Wed, 19 Feb 2025 09:57:04 -0500
Subject: [PATCH] Force comma separated group labels at all times (#7661)

---
 R/context.R                        |  2 +-
 R/utils.R                          |  4 ++--
 tests/testthat/_snaps/context.md   |  2 +-
 tests/testthat/_snaps/summarise.md | 12 +++++++++++-
 tests/testthat/test-summarise.R    |  5 +++++
 5 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/R/context.R b/R/context.R
index 3c3a1d9640..0e0e22d3ee 100644
--- a/R/context.R
+++ b/R/context.R
@@ -72,7 +72,7 @@ cur_group_rows <- function() {
 group_labels_details <- function(keys) {
   keys <- map_chr(keys, pillar::format_glimpse)
   labels <- vec_paste0(names(keys), " = ", keys)
-  labels <- cli_collapse(labels, last = ", ")
+  labels <- cli_collapse(labels, last = ", ", sep2 = ", ")
   cli::format_inline("{.code {labels}}")
 }
 
diff --git a/R/utils.R b/R/utils.R
index e74ac8f32b..3ac4261c62 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -115,8 +115,8 @@ node_walk_replace <- function(node, old, new) {
   }
 }
 
-cli_collapse <- function(x, last = " and ") {
-  cli::cli_vec(x, style = list("vec-last" = last))
+cli_collapse <- function(x, last = " and ", sep2 = " and ") {
+  cli::cli_vec(x, style = list("vec-last" = last, "vec-sep2" = sep2))
 }
 
 with_no_rlang_infix_labeling <- function(expr) {
diff --git a/tests/testthat/_snaps/context.md b/tests/testthat/_snaps/context.md
index 08b03fb5be..a0b60d1fd4 100644
--- a/tests/testthat/_snaps/context.md
+++ b/tests/testthat/_snaps/context.md
@@ -40,5 +40,5 @@
     Code
       group_labels_details(c(a = 1, b = 2))
     Output
-      [1] "`a = 1` and `b = 2`"
+      [1] "`a = 1`, `b = 2`"
 
diff --git a/tests/testthat/_snaps/summarise.md b/tests/testthat/_snaps/summarise.md
index 8040d43a9d..498f4b1107 100644
--- a/tests/testthat/_snaps/summarise.md
+++ b/tests/testthat/_snaps/summarise.md
@@ -124,7 +124,17 @@
       <error/rlang_error>
       Error in `summarise()`:
       i In argument: `a = rlang::env(a = 1)`.
-      i In group 1: `x = 1` and `y = 1`.
+      i In group 1: `x = 1`, `y = 1`.
+      Caused by error:
+      ! `a` must be a vector, not an environment.
+    Code
+      (expect_error(tibble(x = 1, y = c(1, 2, 2), y2 = c(1, 2, 2), z = runif(3)) %>%
+        group_by(x, y, y2) %>% summarise(a = rlang::env(a = 1))))
+    Output
+      <error/rlang_error>
+      Error in `summarise()`:
+      i In argument: `a = rlang::env(a = 1)`.
+      i In group 1: `x = 1`, `y = 1`, `y2 = 1`.
       Caused by error:
       ! `a` must be a vector, not an environment.
     Code
diff --git a/tests/testthat/test-summarise.R b/tests/testthat/test-summarise.R
index 2a0db3b23f..21cad8b804 100644
--- a/tests/testthat/test-summarise.R
+++ b/tests/testthat/test-summarise.R
@@ -477,6 +477,11 @@ test_that("summarise() gives meaningful errors", {
                         group_by(x, y) %>%
                         summarise(a = rlang::env(a = 1))
       ))
+      (expect_error(
+                      tibble(x = 1, y = c(1, 2, 2), y2 = c(1, 2, 2), z = runif(3)) %>%
+                        group_by(x, y, y2) %>%
+                        summarise(a = rlang::env(a = 1))
+      ))
       (expect_error(
                       tibble(x = 1, y = c(1, 2, 2), z = runif(3)) %>%
                         rowwise() %>%