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

Only suggest cli/crayon/fansi #475

Open
hadley opened this issue Aug 29, 2018 · 8 comments
Open

Only suggest cli/crayon/fansi #475

hadley opened this issue Aug 29, 2018 · 8 comments
Labels
feature a feature request or enhancement printing 🖨️
Milestone

Comments

@hadley
Copy link
Member

hadley commented Aug 29, 2018

(Would also need to do the same in pillar)

These are moderately heavy dependencies so it would be nice to make them optional.

I would be cautiously in favour of creating a new package that we could import that suggested cli/crayon/fansi but if weren't available provided a base fallback (i.e. no colour). If you wanted to go that route, I think the best place to start would be a review of all the cli/crayon/fansi functionality currently used in tibble + pillar.

@krlmlr
Copy link
Member

krlmlr commented Aug 29, 2018

Would that new package print a startup message when the suggested packages are not available? Otherwise we might have a tough time debugging problems caused by the parallel code paths.

@hadley
Copy link
Member Author

hadley commented Aug 30, 2018

The parallel code path would do nothing, so I don't think debugging would be hard.

@krlmlr
Copy link
Member

krlmlr commented Sep 5, 2018

I'm not sure. Example: word wrapping. If fansi is available, we use fansi::strwrap_ctl(), otherwise strwrap(). Would we implement it like this?

fallback_strwrap <- function(...) {
  if (color_available()) {
    fansi::strwrap_ctl(...)
  } else {
    strwrap()
  }
}

Perhaps I don't understand what the new package is supposed to handle.

@hadley
Copy link
Member Author

hadley commented Sep 5, 2018

Right - if we're not using colour we don't need special handling for wrapping strings.

The goal is to provide a single package that we can import whenever we want to use colour, but does not import any packages in turn.

@krlmlr
Copy link
Member

krlmlr commented Sep 14, 2018

Minimize cli imports first (convert to Suggests).

@krlmlr
Copy link
Member

krlmlr commented Sep 14, 2018

Then, cli could be used as proxy/shim package.

@krlmlr
Copy link
Member

krlmlr commented Sep 14, 2018

The wrapper interface should use paste0() instead of paste():

> crayon::blue("a", "b", "c")
[1] "\033[34ma b c\033[39m"

Model after usethis::proj_sitrep() .

@krlmlr
Copy link
Member

krlmlr commented Feb 23, 2021

We can't remove fansi yet, only after really removing trunc_mat().

@krlmlr krlmlr modified the milestones: 3.1.0, 4.0.0 Feb 23, 2021
krlmlr added a commit that referenced this issue Feb 23, 2021
- cli and crayon are now suggested packages (#475).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement printing 🖨️
Projects
None yet
Development

No branches or pull requests

2 participants