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.

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

Improve ansi_collapse() consistency #572

Merged
merged 3 commits into from Sep 13, 2023
Merged

Improve ansi_collapse() consistency #572

merged 3 commits into from Sep 13, 2023

Conversation

salim-b
Copy link
Contributor

@salim-b salim-b commented Jan 28, 2023

Before:

cli::ansi_collapse(1:2, trunc = 1, style = "head")
#> [1] "1, and 2"
cli::ansi_collapse(1:2, trunc = 0, style = "head")
#> [1] "1, …"

Created on 2023-01-28 with reprex v2.0.2

After:

cli::ansi_collapse(1:2, trunc = 1, style = "head")
#> [1] "1, and 2"
cli::ansi_collapse(1:2, trunc = 0, style = "head")
#> [1] "1, and 2"

Created on 2023-01-28 with reprex v2.0.2

@gaborcsardi
Copy link
Member

Thanks! The docs say

#' @param trunc Maximum number of elements to show. For `style = "head"`
#' at least `trunc = 1` is used. For `style = "both-ends"` at least
#' `trunc = 5` is used, even if a smaller number is specified.

So according to this, I think

cli::ansi_collapse(1:2, trunc = 1, style = "head")

should show

[1] "1, …"

no? And trunc = 0 should do the same I guess.

@gaborcsardi
Copy link
Member

I realize that a comment currently says

# does not make sense to show ... instead of an element

but I think we can rather follow the documented behavior and show only one element for trunc = 1 (and also for trunc = 0)?

@gaborcsardi
Copy link
Member

@salim-b Thanks, I am going to merge this now, but please let me know if you think my changes are confusing.

@gaborcsardi gaborcsardi merged commit 4fcb4fa into r-lib:main Sep 13, 2023
12 checks passed
@salim-b salim-b deleted the consistency branch September 13, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants