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

Double indentation should be an opt-in/out rule #1157

Closed
mcanouil opened this issue Nov 29, 2023 · 7 comments
Closed

Double indentation should be an opt-in/out rule #1157

mcanouil opened this issue Nov 29, 2023 · 7 comments

Comments

@mcanouil
Copy link

mcanouil commented Nov 29, 2023

Styler "recently" enforces a rule that double indent function declaration.
Let's say you have the following.

f <- function(
  x,
  y
) {
  "content"
}

It will be "messed up" (sorry, personal opinion on double indentation) as:

f <- function(
    x,
    y) {
  "content"
}

I know there is #1137 open for few months now, but regardless of this PR, I do believe double indent rule should be as other rules an opt-in/out rule.

Thanks for considering this.

@lorenzwalthert
Copy link
Collaborator

Hi @mcanouil. {styler} implements the tidyverse style guide and your code does not correspond to that style. As you can read in various issues, I am not the biggest fan of double indent, but there is another way

function(x = 1, 
         y = 2) {
  NULL
}

These are the two officially supported styles and styler adheres to those.

@mcanouil
Copy link
Author

mcanouil commented Nov 29, 2023

I know that's a "new" tidyverse style guide rule, but that is not the point.

My question is why is it not controlled by a rule/transformer that can be opt-out? https://styler.r-lib.org/articles/remove_rules.html

EDIT: the rule/transformer regarding function declaration all uses is_double_indent_function_declaration(), seems pretty simple to offer a parameter to disable this or not, as it is possible to use a 12 spaces indentation.

For now, I was basically trashing the whole set of transformers:

styler::style_text(
  text = "
    f <- function(
    x,
    y
  ) {
    NULL
  }",
  style = function(...) {
    transformers <- styler::tidyverse_style(...)
    transformers$indention$update_indention_ref_fun_dec <- NULL
    transformers$indention$unindent_fun_dec <- NULL
    transformers$line_break$remove_line_breaks_in_fun_dec <- NULL
    transformers
  }
)

@lorenzwalthert
Copy link
Collaborator

I am happy to review and merge a PR that refactors existing rules such that the removal of the double indention is possible in an easier way, e.g. by simply remove the rule, but I won't contribute this change.

@lorenzwalthert
Copy link
Collaborator

The answer to why it is not already implemented like this is because probably I did not care or it was more complex.

@mcanouil
Copy link
Author

I see.
I leave up to you to close or not this issue. Thanks for the quick reply.

@lorenzwalthert
Copy link
Collaborator

Sure. Thanks @mcanouil for understanding that catering to all user requests here is difficult and my time is limited since no one pays me to work on {styler}.

@mcanouil
Copy link
Author

my time is limited since no one pays me to work on {styler}.

I know this too well.
That's the usual thing in packages development 😅

Anyway, thanks for the time spent on the package and replying. 😉

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

No branches or pull requests

2 participants