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

styler.equals::style_pkg() makes fewer corrections than styler::style_pkg() #3

Closed
eitsupi opened this issue May 13, 2023 · 6 comments · May be fixed by #4
Closed

styler.equals::style_pkg() makes fewer corrections than styler::style_pkg() #3

eitsupi opened this issue May 13, 2023 · 6 comments · May be fixed by #4

Comments

@eitsupi
Copy link

eitsupi commented May 13, 2023

From pola-rs/r-polars#214

Thanks for working on this.
I tried styler.equals::style_pkg() on the polars package, but it was necessary to run it after styler::style_pkg() because the modifications were not made as expected.
Are differences other than assign operators intentional?
Or do you plan to make all but the assignment operator equivalent to styler::style_pkg() in the future?

@Robinlovelace
Copy link
Owner

Or do you plan to make all but the assignment operator equivalent to styler::style_pkg() in the future?

The intention was for everything to be equivalent now. Indeed that's what I thought would happen based on what's in this package. Can you provide more detail on what did not change with this package vs styler? Thanks for reporting and sorry for slow response btw.

@Robinlovelace
Copy link
Owner

OK update on this:

Currently, all the style guide is doing is to replace <- with =

From https://github.com/lorenzwalthert/styler.yours README (I had incorrectly assumed tidyverse as default).

So work to do!

@Robinlovelace
Copy link
Owner

Not that much work to do... It's to change this

styler.equals/R/core.R

Lines 15 to 56 in d4e4fc9

equals_style = function(scope = "tokens",
strict = TRUE,
indent_by = 2,
start_comments_with_one_space = FALSE,
reindention = tidyverse_reindention(),
math_token_spacing = tidyverse_math_token_spacing()) {
args = as.list(environment())
scope = styler::scope_normalize(scope)
indention_manipulators = if ("indention" %in% scope) {
list()
}
space_manipulators = if ("spaces" %in% scope) {
list()
}
use_raw_indention = !("indention" %in% scope) || length(indention_manipulators) < 1
line_break_manipulators = if ("line_breaks" %in% scope) {
list()
}
token_manipulators = if ("tokens" %in% scope) {
list(force_assignment_eq = force_assignment_eq)
}
create_style_guide(
# transformer functions
initialize = default_style_guide_attributes,
line_break = line_break_manipulators,
space = space_manipulators,
indention = indention_manipulators,
token = token_manipulators,
# transformer options
use_raw_indention = use_raw_indention,
reindention = reindention,
style_guide_name = "styler.equals::equals_style@https://github.com/robinlovelace/styler.equals/",
style_guide_version = version,
more_specs_style_guide = args
)
}

So it's more like

https://github.com/r-lib/styler/blob/8e9ac82333c2a0036a1653a029b6437008650f34/R/style-guides.R#L65-L257

Any ideas/help with that v. welcome, I guess a reasonable starting point would be try the whole tidyverse. Personally I'd like a few tweaks compared with the tidyverse, favouring if( over if ( for example but happy to go in whichever direction will be most use to the most people.

@eitsupi
Copy link
Author

eitsupi commented May 21, 2023

Thanks for the reply. Perhaps even keep a few different styles in the package?

@Robinlovelace
Copy link
Owner

Finally looking at this...

@Robinlovelace
Copy link
Owner

I think this is fixed now..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants