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

New Cop Style/ClosingArrayBraceLayout #2472

Merged
merged 1 commit into from
Dec 27, 2015

Conversation

panthomakos
Copy link
Contributor

This cop is inspired by a comment in the Style/LineBreaks cop PR:

#2277 (comment)

This cop checks that the closing brace in an array is symmetrical with
respect to the opening brace and the array elements. I am very open to
suggestions on naming and ways to clarify the offense messages.

# bad
[ :a,
  :b
]

# bad
[
  :a,
  :b ]

# good
[ :a,
  :b ]

#good
[
  :a,
  :b
]

My intention is to expand this to account for hashes, method parameters
and method arguments once the language and approach is validated.

@panthomakos panthomakos force-pushed the closing-array-brace-layout branch 2 times, most recently from c47391d to 94a0237 Compare December 7, 2015 21:23
@panthomakos panthomakos force-pushed the closing-array-brace-layout branch 2 times, most recently from 76b2446 to a19a3ae Compare December 10, 2015 19:34
@bbatsov
Copy link
Collaborator

bbatsov commented Dec 10, 2015

Technically speaking this is more of a "multi-line array literal layout". A single brace can't have a layout. :-)

@panthomakos
Copy link
Contributor Author

@bbatsov Maybe the name of the cop should be Style/MultilineArrayBraceLayout or Style/MultilineArrayLiteralLayout? How is the wording of the description?

@@ -4,6 +4,12 @@ Style/AutoResourceCleanup:
Description: 'Suggests the usage of an auto resource cleanup version of a method (if available).'
Enabled: false

Style/ClosingArrayBraceLayout:
Description: >-
Checks that the closing brace in an array is symmetrical
Copy link
Collaborator

Choose a reason for hiding this comment

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

in an array literal

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 20, 2015

I'm fine with both names.

'from the first array element.'

def on_array(node)
return unless node.loc.begin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Normally I add comments on guard clauses, so casual readers would know why it's good idea to exit at this point.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 25, 2015

ping

@panthomakos
Copy link
Contributor Author

Holiday/vacation. I promise I'll get back to this.

On Fri, Dec 25, 2015, 01:20 Bozhidar Batsov notifications@github.com
wrote:

ping


Reply to this email directly or view it on GitHub
#2472 (comment).

@panthomakos
Copy link
Contributor Author

@bbatsov I've updated with your comments and renamed to Style/MultilineArrayBraceLayout.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 27, 2015

The build is failing due to a style offense.

This cop is inspired by a comment in the `Style/LineBreaks` cop PR:

rubocop#2277 (comment)

This cop checks that the closing brace in an array literal is symmetrical
with respect to the opening brace and the array elements. I am very open
to suggestions on naming and ways to clarify the offense messages.

    # bad
    [ :a,
      :b
    ]

    # bad
    [
      :a,
      :b ]

    # good
    [ :a,
      :b ]

    #good
    [
      :a,
      :b
    ]

My intention is to expand this to account for hashes, method parameters
and method arguments once the language and approach is validated.
@panthomakos
Copy link
Contributor Author

@bbatsov fixed

bbatsov added a commit that referenced this pull request Dec 27, 2015
@bbatsov bbatsov merged commit 65dfb1e into rubocop:master Dec 27, 2015
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