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

Enable symmetrical braces for methods #3091

Merged
merged 1 commit into from
Apr 28, 2016

Conversation

panthomakos
Copy link
Contributor

@panthomakos panthomakos commented Apr 28, 2016

Related to #2914 this change enables
Style/MultilineMethodCallBraceLayout and
Style/MultilineMethodDefinitionBraceLayout with EnforcedStyle: symmetrical by default.

All existing RuboCop code has been auto-corrected.

@@ -42,9 +42,7 @@ def on_or(node)
receiver: receiver.source,
method: method,
combined_args: combine_args(first_call_args, second_call_args),
original_code: node.source
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems there's a bug in the cop. When there's no element on the first line, the current indentation seems to me to be the preferable one.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 28, 2016

Hopefully you understand my issue with current behaviour. For me:

something(
  a,
  b,
  c
)

is also symmetrical, as there's no element on the first line after (.

@panthomakos
Copy link
Contributor Author

@bbatsov I agree with you and I think that symmetrical is a better default for method calls and definitions. I was following what I thought was your and @jonas054's preference from #2914 (hashes and arrays use symmetrical and method calls and definitions use same_line). But I am happy to make the default symmetrical for method calls and definitions as well if that is your preference.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 28, 2016

But I am happy to make the default symmetrical for method calls and definitions as well if that is your preference.

Well, as I said before - it depends for me on the line length.

Normally I write method calls like this:

method(something,
             else,
             top)

but when I'm short on space I go for:

method(
  something,
  else,
  top
)

Ideally there should be a style that's aware whether the first elem is on the same line as the opening paren and determine from it where the closing paren should be.

@panthomakos
Copy link
Contributor Author

This cop does exactly that w/ the symmetrical style. I have updated the PR to use the symmetrical instead of the same_line style. PTAL

@panthomakos panthomakos changed the title Enable same_line braces for methods Enable symmetrical braces for methods Apr 28, 2016
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 28, 2016

Apart from the failing build - the code looks good to me now.

Related to rubocop#2914 this change enables
`Style/MultilineMethodCallBraceLayout` and
`Style/MultilineMethodDefinitionBraceLayout` with `EnforcedStyle:
symmetrical` by default.

All existing RuboCop code has been auto-corrected.
@panthomakos
Copy link
Contributor Author

Added one too many lines to one module. Fixed!

@bbatsov bbatsov merged commit e63f764 into rubocop:master Apr 28, 2016
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 28, 2016

👍

Neodelf pushed a commit to Neodelf/rubocop that referenced this pull request Oct 15, 2016
Related to rubocop#2914 this change enables
`Style/MultilineMethodCallBraceLayout` and
`Style/MultilineMethodDefinitionBraceLayout` with `EnforcedStyle:
symmetrical` by default.

All existing RuboCop code has been auto-corrected.
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 this pull request may close these issues.

2 participants