-
Notifications
You must be signed in to change notification settings - Fork 73
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
Add equals_style() #451
Add equals_style() #451
Conversation
Copied-over from #449 |
To do after merge: reorganize file style-guides into smaller files. |
#' | ||
#' @details | ||
#' | ||
#' This style guide is the same as [`tidyverse_style()`], except it uses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can drop the backticks. in tidyverse_style()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description is not true with the current implementation, as it does not inherit from the tidyverse style guide. Can you adapt that?
#' | ||
#' Use equals assignment instead of arrow assignment. | ||
#' | ||
#' @inheritParams tidyverse_style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to inherit from the tidyverse style guide? I don't see how they are related.
Sorry @Robinlovelace for some reason these comments were just posted now because I clicked start a review and never finished instead of just commenting one-off. I wrote them a few weeks ago :-( |
Hi @lorenzwalthert thanks for the comments. I also just saw these! Will aim to look this evening but not guarantees... |
Because of #513, I am not sure if we should ever replace |
Yes in edge cases were gsub(pattern = "<-", replacement = "=", x = "x <- mean(cars$speed, na.rm = TRUE)") works. Still I think it would be great to support styling to equals assignement. |
Well, it's rarely a good idea to work with regular expressions I think. It's rather slow, unreliable and does not fit the styler framework. We found a solution to #513 (just see the discussion there) and implmenented in this way: pat-s@5bf20ab. To use |
Great. Fully agree a non-regex solution would be ideal. Not sure how it relates to this PR though, I've lost track of where we were and next steps I'm afaid! |
Closed due to inactivity. |
Sensible decision, would you welcome a rejuvenation of this? Would be useful for ppl using the style in their work. I've been using Would be great to be able to go in the opposite direction, e.g. here: https://github.com/ropensci/stats19/blob/master/inst/iow_example.R |
Sorry, I did not mean you to delete the branch entirely. I should probably have asked about the status of this before closing. Sorry. Yes, you can create another PR. |
OK great. I think starting over (again!) may be the best way forward in general, but I'm sure of the specific next steps. Will see if there is scope to combine efforts with @pat-s on this. As always any pointers/thoughts welcome and no worries about closing the PR, it had definitely gone stale! |
@Robinlovelace If you refer to the "mlr-style", @lorenzwalthert did most of the work. I essentially just tested and used it since then :) (and bother him once in a while with some upstream merge issues 😄 ) |
Rebased from master.