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

Restyle expression-derived source with styler #94

Closed
jennybc opened this issue Aug 1, 2017 · 9 comments
Closed

Restyle expression-derived source with styler #94

jennybc opened this issue Aug 1, 2017 · 9 comments

Comments

@jennybc
Copy link
Member

@jennybc jennybc commented Aug 1, 2017

Expression input generally gets processed with formatR::tidy_source() due to the addition of this line in the .R source file:

knitr::opts_chunk$set(tidy = TRUE, tidy.opts = list(indent = 2))

But this does not play well with the bang-bang syntactic sugar from tidyeval:

formatR::tidy_source(
  text = c(
    "nameshift <- c(SL = 'Sepal.Length')",
    "head(dplyr::rename(iris[,1:2], !!!nameshift), 3)"
  )
)
#> nameshift <- c(SL = "Sepal.Length")
#> head(dplyr::rename(iris[, 1:2], !(!(!nameshift))), 3)

styler is under development and its non-invasive quality works to our advantage:

styler::style_text(
  c(
    "nameshift <- c(SL = 'Sepal.Length')",
    "head(dplyr::rename(iris[,1:2], !!!nameshift), 3)"
  )
)
#> [1] "nameshift <- c(SL = \"Sepal.Length\")"            
#> [2] "head(dplyr::rename(iris[, 1:2], !!!nameshift), 3)"

Switch to that once it's on CRAN.

cc @krlmlr @lorenzwalthert

@lorenzwalthert
Copy link
Contributor

@lorenzwalthert lorenzwalthert commented Aug 1, 2017

Glad to see it works for you. We haven't payed particular attention to tidyeval / rlang features yet, so let us know if you find something missing / wrongly formatted. A CRAN submission is planned for late August.

Loading

@jennybc
Copy link
Member Author

@jennybc jennybc commented Aug 2, 2017

How firm is your timeline @lorenzwalthert? If it's pretty good, I could switch over now and have dev reprex depend on dev styler, which might allow a few of us to accumulate a bit of reprex + styler + tidyeval experience w/o even trying.

Loading

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Aug 3, 2017

We should release to CRAN as part of the GSoC project, we have no reason to expect major delays. It would be awesome to give styler a bit of exposure before that, but I remember a few minor problems we wanted to fix before announcing a stable-ish version. @lorenzwalthert: Can you please comment?

Loading

@lorenzwalthert
Copy link
Contributor

@lorenzwalthert lorenzwalthert commented Aug 3, 2017

Yes, @krlmlr is right. We fixed all remaining (minor) problems now except for token-dependent indention (i.e. indention that is not just the regular 2, 4, etc. spaces but depends on the length of other tokens). That means styler will format

a <- function(x,
              y) {
}

and the like as

a <- function(x,
  y) {
}

However we should also be able to fix that (see PR #104) soon, so we will submit to CRAN by the end of August.

Loading

@jennybc
Copy link
Member Author

@jennybc jennybc commented Aug 3, 2017

So we can just start using it here with no fanfare, knowing we won't have a non-CRAN dependency for long. I will leave any active publicity to you two.

Loading

@lorenzwalthert
Copy link
Contributor

@lorenzwalthert lorenzwalthert commented Aug 3, 2017

Ok, great.

Loading

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Aug 24, 2017

I think we could give styler a try now, it seems to work well and we're about to submit to CRAN. Should we use styler exclusively, or keep formatR as an option?

Loading

@jennybc
Copy link
Member Author

@jennybc jennybc commented Aug 24, 2017

I lean towards just switching over to styler.

I am trying to get googledrive submitted to CRAN and can do some maintenance here right after.

Loading

@jennybc
Copy link
Member Author

@jennybc jennybc commented Sep 26, 2017

Re-styling is no longer mandatory for expression input, so closing this.

Re-styling with styler will, instead, be made available for all input modes: #108.

Loading

@jennybc jennybc closed this Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants