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

Make style = TRUE the default? #208

Closed
krlmlr opened this issue Sep 15, 2018 · 7 comments
Closed

Make style = TRUE the default? #208

krlmlr opened this issue Sep 15, 2018 · 7 comments
Labels
feature a feature request or enhancement options tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day

Comments

@krlmlr
Copy link
Member

krlmlr commented Sep 15, 2018

This might make the life of package maintainers a tad easier. Two things to remember:

  • styler throws an unhelpful error message if the code can't be parsed, we might want to do better to help users.
  • Only scope = "line_breaks" guarantees that the input is identical to the output as far as the parser is concerned. This will keep = for assignment and won't add () after function calls in pipes but is the safer alternative.

CC @lorenzwalthert.

@lorenzwalthert
Copy link
Contributor

There's been a debate about whether to use style = TRUE as a default in #108. Personally, I think it would make sense, but there is also an easy way to make reprex calls behaving as if style = TRUE was the default, which I use: Just use the global option to change the default of the argument:

options(reprex.style = TRUE)

The thing is probably that most users don't do that or know how to do it.

@batpigandme
Copy link
Contributor

batpigandme commented Sep 15, 2018

FWIW, I think (depending on how different an individual's coding style is from styler) this could be kind of confusing to the unsuspecting user.

I'm thinking more about Q&A than GitHub issues, I put my code in, and now it looks all different!

@krlmlr
Copy link
Member Author

krlmlr commented Sep 15, 2018

Ah, that's what the opt() does ;-)

reprex could talk about it, like:

Code formatted by styler. To turn off, set options(reprex.style) = FALSE.

Keeping this open, because the original discussion is now a year old and styler has matured a lot since then.

@jennybc
Copy link
Member

jennybc commented Sep 15, 2018

I'm certainly open to this but won't do anything for the small patch release I want to make in the very near future. It would be nice if reprex offered more support, via an add-in or helper function, for people who want to customize package-level options. We could be more bold with defaults if we had that.

@jennybc
Copy link
Member

jennybc commented May 17, 2019

Maybe style should default to NA, with default logic of: set style = TRUE if styler is installed.

This would need to be implemented and is also connected to centralized documentation for options: #259

@jennybc jennybc added feature a feature request or enhancement options tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day labels May 17, 2019
@lorenzwalthert
Copy link
Contributor

I know this is for interactive use primarily but isn't it generally considered bad practice to have code behaving differently depending on what other packages are installed? Do you plan to have similar behavior for other options?

@jennybc
Copy link
Member

jennybc commented May 18, 2019

Well, there is precedent for this sort of thing for a worklow / interactive package. For example, httr and everything which depends on it uses a local web server for auth (vs out-of-bound auth) depending on whether httpuv is installed.

I don't think it's a big deal either way.

But we can also leave style = FALSE as the default.

@jennybc jennybc closed this as completed May 18, 2019
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 options tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day
Projects
None yet
Development

No branches or pull requests

4 participants