r-lib / cli

Tools for making beautiful & useful command line interfaces
https://cli.r-lib.org/
Other
625 stars 66 forks source link

Bad formatting when passing bare lubdridate dates #665

Closed DanChaltiel closed 2 weeks ago

DanChaltiel commented 5 months ago

Hi,

Here is a very minor bug that happens when passing bare dates to cli. An easy workaround is wrapping the value in curly braces.

Here is a reprex with cli_warn():

x = lubridate::today()
#or x = base::Sys.Date()
cli::cli_warn(x)
#> Warning: 19739
cli::cli_warn(c("foobar", i=x))
#> Warning: foobar
#> i 19739
cli::cli_warn("{x}")
#> Warning: 2024-01-17

Created on 2024-01-17 with reprex v2.0.2

gaborcsardi commented 5 months ago

Maybe we could fix this, but in general you should always pass string literals to cli functions.

DanChaltiel commented 5 months ago

Then, maybe a simple stopifnot(is.character(message)) would solve the problem? Or parsing message as character?

gaborcsardi commented 5 months ago

I don't mean a character object, I mean a literal. I.e. not

x <- "foobar"
cli_text(x)

but

cli_text("foobar")

Btw. c("foobar", i=x) is not a date any more, so there is no way to recover from that:

❯ c("foobar", i = x)
                i 
"foobar"  "19739" 
DanChaltiel commented 5 months ago

Oh yes, of course it is not a date anymore, how could I miss that!

OK, I get it, this makes perfect sense. Indeed, I don't think I ever really use cli without a literal, this just happened during debugging (yes, I shamelessly use cli as a debug printer).

Parsing message to character seems to solve the initial date problem (which is not limited to lubridate as I initially thought).

BTW, I am surprised that all objects are allowed and printed bare:

x = lm(1~1)
cli::cli_warn(x) #cli_text wraps it
#> Warning: 1
#> 0
#> 1
#> 1
#> 1
#> 0
#> 1111e-071
#> 0
#> lm(formula = 1 ~ 1)
#> 1 ~ 1
#> 1

Created on 2024-01-17 with reprex v2.0.2

Mostly, cli::cli_text(iris) would print the whole dataset, which would crash my console if run by error with a very large table. But I'm drifting out of the scope of this issue.