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

"{}" causes ic() to break #26

Closed
notPlancha opened this issue Feb 3, 2024 · 3 comments · Fixed by #28
Closed

"{}" causes ic() to break #26

notPlancha opened this issue Feb 3, 2024 · 3 comments · Fixed by #28
Assignees
Labels
bug Something isn't working

Comments

@notPlancha
Copy link

notPlancha commented Feb 3, 2024

Because of glue (and cli), any string that includes the brackets will break, which I feel it's not intentional, since I think I'd want the contents of the variable I'm seeing (similar to print). I first noticed it when working with json.

library(icecream)
ic("{")     # Error 
ic("{p}")  # Error 
var <- "{printthis}"
ic(var)
var2 <- ic(paste("{", "anotherthing")) #Error
jsonexample <- "{'key1': 'value1'}"
ic(jsonexample) #Error

I don't think this is intended, and if it is I think there should be a note about this (or an option to not use it).

Depending on how ic.output.function is implemented, the reimplementation could be fixed. Until then, I suggest adding to print.R:

output <- paste(prefix, output)
output <- gsub("\\{", "{{", output)
cli::cli_alert_info(output) # TODO: This is ...
escape_glue <- \(x) cli::cli_text(gsub("\\{", "{{", x))
escape_glue("{")
escape_glue("{}")
escape_glue("{printthis}")
escape_glue("{'key1': 'value1'}")
escape_glue(paste("{", "anotherthing"))
escape_glue("No brackets")

I can create a pull request if you think the suggestion is fine (and enough)

edit: just remembered other types that are not character so what I was recommending is not enough

@lewinfox lewinfox self-assigned this Feb 17, 2024
@lewinfox lewinfox added the bug Something isn't working label Feb 17, 2024
lewinfox added a commit that referenced this issue Feb 17, 2024
Some messing about with escaping glue code has been necessary. See
GitHub issue for details of the problem.

#26
@lewinfox lewinfox mentioned this issue Feb 17, 2024
@lewinfox
Copy link
Owner

@notPlancha really appreciate you taking the time to flag this, and to submit the PR. As mentioned in my comment on #27 I don't want to apply a blanket find-and-replace, so I've gone for "try it, if you hit an error escape the curlies". I've also added a couple of unit tests based on the examples you provided.

Would you be interested in doing a bit of testing? If so you can devtools::install_github("lewinfox/icecream", "bug/curlies"). Any feedback would be gratefully received.

If I don't hear from you I'll look to do a release in the next week or so.

@notPlancha
Copy link
Author

looks good to me

lewinfox added a commit that referenced this issue Feb 17, 2024
@lewinfox
Copy link
Owner

@notPlancha just released v0.2.2 to CRAN with this fix. Any further issues let me know. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants