-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
Replace crayon by cli + address some TODOs to add some color #4170
Conversation
Are there any performance changes in what's currently in the PR? |
Sorry, I should have been clearer. I didn't include the change in question in this PR. I was just storing this unrelated info here to make sure I'd find it when I needed it. |
That makes sense; it's also how I interpreted your notes. In other words, since you had looked at the speed, it'd be helpful to know if there are any performance changes in the PR as it is now. Also, if you're still working on this and aren't ready for a review, would you mind changing the PR to a draft to avoid confusion? |
No, I am done with this PR! I observed decreased performance with some other changes I didn't include in the PR (that I may send as a follow-up if I figure out how to limit decreased performance) Here are some bench marks for this PR. # Simple color
# cli more performant!
bench::mark(
x1 = crayon::silver("xx"),
x2 = cli::col_silver("xx"),
check = FALSE
)
#> # A tibble: 2 × 6
#> expression min median `itr/sec` mem_alloc `gc/sec`
#> <bch:expr> <bch:tm> <bch:tm> <dbl> <bch:byt> <dbl>
#> 1 x1 68.4µs 147µs 5313. 172.4KB 2.03
#> 2 x2 22.7µs 25µs 30405. 76.4KB 3.04
# Bold: no performance lost
bench::mark(
x1 = crayon::blue$bold("xx"),
x2 = cli::style_bold(cli::col_blue("xx")),
check = FALSE
)
#> # A tibble: 2 × 6
#> expression min median `itr/sec` mem_alloc `gc/sec`
#> <bch:expr> <bch:tm> <bch:tm> <dbl> <bch:byt> <dbl>
#> 1 x1 73.7µs 117.8µs 8392. 30.3KB 6.21
#> 2 x2 42.7µs 70.9µs 14126. 44KB 4.09
# cat vs cli::cat_bullet().
# slight performance decrease
# but still fast (26 microseconds vs 336 microseconds)
bench::mark(
x1 = cat("* Success\n"),
x2 = cli::cat_bullet("Success", bullet = "tick", bullet_col = "green")
)
#> # A tibble: 2 × 6
#> expression min median `itr/sec` mem_alloc `gc/sec`
#> <bch:expr> <bch:tm> <bch:tm> <dbl> <bch:byt> <dbl>
#> 1 x1 18.3µs 22.5µs 38086. 11.5KB 3.81
#> 2 x2 336µs 421µs 1893. 27.2KB 4.14 Created on 2025-01-20 with reprex v2.1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @olivroy and thanks for including the extra benchmarking info!
Also removed RdMacros since no longer used. (also no longer recommended by pkgdown..
The end goal is to have clickable links in output.
info (to ignore for now)
With a change I have locally (not included here), I am able to acheive the following for the stack trace. However, there is a slowdown. So, I will look into it.