Skip to content

Conversation

MichaelChirico
Copy link
Collaborator

Part of #962

We might consider this one to be a default linter btw, it's basically part of the style guide.

@AshesITR
Copy link
Collaborator

AshesITR commented Mar 22, 2022

In favor of making this a default linter, although I don't see explicit mention of this rule in https://style.tidyverse.org/syntax.html#control-flow
@jimhester, @hadley WDYT? Should we lint

if (test) a else {
  long_code
}

if (test) {
  long_code
} else a

by default?

@MichaelChirico
Copy link
Collaborator Author

FWIW styler agrees:

styler::style_text('if (test) a else {
  long_code
}

if (test) {
  long_code
} else a
')
# if (test) {
#   a
# } else {
#   long_code
# }

# if (test) {
#   long_code
# } else {
#   a
# }

@hadley
Copy link
Member

hadley commented Mar 23, 2022

Yeah this is definitely required by tidyverse style.

@MichaelChirico
Copy link
Collaborator Author

OK, done

AshesITR
AshesITR previously approved these changes Mar 23, 2022
Copy link
Collaborator

@AshesITR AshesITR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AshesITR AshesITR merged commit eefe67b into master Mar 24, 2022
@MichaelChirico MichaelChirico deleted the if_else_match_braces branch March 30, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants