Skip to content

Commit 232b24d

Browse files
committed
Restore original quo_name() behaviour
Use label() to create default names. Currently unexported. Part of #636
1 parent 3d7f95d commit 232b24d

File tree

6 files changed

+65
-30
lines changed

6 files changed

+65
-30
lines changed

R/expr.R

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,45 @@ expr_label <- function(expr) {
154154
#' @rdname expr_label
155155
#' @export
156156
expr_name <- function(expr) {
157-
switch(typeof(expr),
157+
switch_type(expr,
158158
NULL = "NULL",
159159
symbol =
160160
if (is_missing(expr)) {
161161
"<empty>"
162162
} else {
163163
as_string(expr)
164164
},
165+
quosure = ,
166+
language =
167+
if (is_data_pronoun(expr)) {
168+
data_pronoun_name(expr) %||% "<unknown>"
169+
} else {
170+
name <- deparse_one(expr)
171+
name <- gsub("\n.*$", "...", name)
172+
name
173+
},
174+
if (is_scalar_atomic(expr)) {
175+
# So 1L is translated to "1" and not "1L"
176+
as.character(expr)
177+
} else if (length(expr) == 1) {
178+
name <- expr_text(expr)
179+
name <- gsub("\n.*$", "...", name)
180+
name
181+
} else {
182+
abort("`expr` must quote a symbol, scalar, or call")
183+
}
184+
)
185+
}
186+
label <- function(expr) {
187+
expr <- quo_squash(expr)
188+
189+
if (is_missing(expr)) {
190+
return("<empty>")
191+
}
192+
193+
switch(typeof(expr),
194+
NULL = "NULL",
195+
symbol = as_string(expr),
165196
language = {
166197
if (is_data_pronoun(expr)) {
167198
data_pronoun_name(expr) %||% "<unknown>"

R/quo.R

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -398,21 +398,8 @@ quo_text <- function(quo, width = 60L, nlines = Inf) {
398398
#' @rdname quo_label
399399
#' @export
400400
quo_name <- function(quo) {
401-
expr <- quo_squash(quo)
402-
if (is_missing(expr)) {
403-
return("<empty>")
404-
}
405-
if (quo_name_needs_compat &&
406-
is_scalar_atomic(expr) &&
407-
is_reference(env_parent(caller_env()), ns_env("tidyr"))) {
408-
return(as.character(expr))
409-
}
410-
expr_name(expr)
401+
expr_name(quo_squash(quo))
411402
}
412-
# COMPAT: tidyr 0.8.1
413-
delayedAssign("quo_name_needs_compat", {
414-
is_installed("tidyr") && utils::packageVersion("tidyr") <= "0.8.1"
415-
})
416403

417404
quo_squash_impl <- function(x, parent = NULL, warn = FALSE) {
418405
switch_expr(x,

R/quotation.R

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,11 +425,10 @@ exprs_auto_name <- function(exprs, width = NULL, printer = NULL) {
425425
"The `printer` argument is soft-deprecated as of rlang 0.3.0."
426426
))
427427
}
428-
printer <- quo_name
429428

430429
have_name <- have_name(exprs)
431430
if (any(!have_name)) {
432-
nms <- map_chr(exprs[!have_name], printer)
431+
nms <- map_chr(exprs[!have_name], label)
433432
names(exprs)[!have_name] <- nms
434433
}
435434

R/trace.R

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,9 +596,10 @@ trace_as_tree <- function(x, dir = getwd(), srcrefs = NULL) {
596596
tree
597597
}
598598

599+
# FIXME: Add something like call_deparse_line()
599600
trace_call_text <- function(call, collapse) {
600601
if (is_null(collapse)) {
601-
return(expr_name(call))
602+
return(label(call))
602603
}
603604

604605
if (is_call(call, "%>%")) {
@@ -607,7 +608,7 @@ trace_call_text <- function(call, collapse) {
607608
call <- call2(node_car(call), quote(...))
608609
}
609610

610-
text <- expr_name(call)
611+
text <- label(call)
611612
if (collapse > 0L) {
612613
n_collapsed_text <- sprintf(" ... +%d", collapse)
613614
} else {

tests/testthat/test-deparse.R

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ test_that("needs_backticks() detects non-syntactic symbols", {
318318
test_that("expr_text() and expr_name() interpret unicode tags (#611)", {
319319
expect_identical(expr_text(quote(`<U+006F>`)), "o")
320320
expect_identical(expr_name(quote(`~f<U+006F><U+006F>`)), "~foo")
321+
expect_identical(label(quote(`~f<U+006F><U+006F>`)), "~foo")
321322
})
322323

323324
test_that("expr_text() deparses non-syntactic symbols with backticks (#211)", {
@@ -336,6 +337,7 @@ test_that("expr_name() deparses empty arguments", {
336337
expect_identical(expr_name(expr()), "<empty>")
337338
expect_identical(quo_name(quo()), "<empty>")
338339
expect_identical(names(quos_auto_name(quos(, ))), "<empty>")
340+
expect_identical(label(expr()), "<empty>")
339341
})
340342

341343
test_that("expr_deparse() handles newlines in strings (#484)", {
@@ -360,7 +362,9 @@ test_that("expr_deparse() handles ANSI escapes in strings", {
360362
expect_identical(expr_deparse("\\0"), deparse("\\0"))
361363
})
362364

363-
test_that("expr_name() handles .data pronoun", {
365+
test_that("label() and expr_name() handles .data pronoun", {
364366
expect_identical(expr_name(quote(.data[["bar"]])), "bar")
365367
expect_identical(quo_name(quo(.data[["bar"]])), "bar")
368+
expect_identical(label(quote(.data[["bar"]])), "bar")
369+
expect_identical(label(quo(.data[["bar"]])), "bar")
366370
})

tests/testthat/test-expr.R

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,22 +47,35 @@ test_that("expr_label() truncates long calls", {
4747

4848
# expr_name() --------------------------------------------------------
4949

50-
test_that("name symbols, calls, and literals", {
50+
test_that("expr_name() with symbols, calls, and literals", {
5151
expect_identical(expr_name(quote(foo)), "foo")
5252
expect_identical(expr_name(quote(foo(bar))), "foo(bar)")
53-
expect_identical(expr_name(1L), "1L")
54-
expect_identical(expr_name("foo"), "\"foo\"")
53+
expect_identical(expr_name(1L), "1")
54+
expect_identical(expr_name("foo"), "foo")
5555
expect_identical(expr_name(function() NULL), "function () ...")
5656
expect_identical(expr_name(expr(function() { a; b })), "function() ...")
57-
expect_identical(expr_name(1:2), "<int>")
58-
expect_identical(expr_name(env()), "<env>")
57+
expect_identical(expr_name(NULL), "NULL")
58+
expect_error(expr_name(1:2), "must quote")
59+
expect_error(expr_name(env()), "must quote")
5960
})
6061

61-
test_that("supports special objects", {
62-
expect_match(expr_name(quote(foo := bar)), ":=")
63-
expect_identical(expr_name(quo(foo)), "~foo")
64-
expect_identical(expr_name(~foo), "~foo")
65-
expect_identical(expr_name(NULL), "NULL")
62+
test_that("label() with symbols, calls, and literals", {
63+
expect_identical(label(quote(foo)), "foo")
64+
expect_identical(label(quote(foo(bar))), "foo(bar)")
65+
expect_identical(label(1L), "1L")
66+
expect_identical(label("foo"), "\"foo\"")
67+
expect_identical(label(function() NULL), "function () ...")
68+
expect_identical(label(expr(function() { a; b })), "function() ...")
69+
expect_identical(label(1:2), "<int>")
70+
expect_identical(label(env()), "<env>")
71+
})
72+
73+
test_that("label() supports special objects", {
74+
expect_match(label(quote(foo := bar)), ":=")
75+
expect_identical(label(quo(foo)), "foo")
76+
expect_identical(label(quo(foo(!!quo(bar)))), "foo(bar)")
77+
expect_identical(label(~foo), "~foo")
78+
expect_identical(label(NULL), "NULL")
6679
})
6780

6881

0 commit comments

Comments
 (0)