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: MultilineBlockLayout #1079

Merged
merged 1 commit into from
Jul 17, 2014

Conversation

barunio
Copy link
Contributor

@barunio barunio commented May 10, 2014

This PR adds a new cop that was extracted from the code in #1002. It checks for the situation where a multiline block contains an inner expression on the same line as the start of the block. It includes autocorrection.

module Style
# This cop checks whether the multiline do end blocks have a newline
# after the start of the block.
#
Copy link
Collaborator

Choose a reason for hiding this comment

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

Strictly speaking, same rule should apply for both do/end and {} blocks, so I'd suggest extending the examples here and in the tests.

@bbatsov
Copy link
Collaborator

bbatsov commented May 11, 2014

There is a similar cop named ElseLayout, so I think we should name this MultilineBlockLayout for consistency.

And you'll need to add a changelog entry.

@jonas054 jonas054 self-assigned this Jul 17, 2014
@barunio barunio changed the title New cop: NewlineAfterMultilineBlockStart New cop: MultilineBlockLayout Jul 17, 2014
@barunio
Copy link
Contributor Author

barunio commented Jul 17, 2014

@bbatsov I believe I've adjusted for all of your comments except the one about removing trailing spaces on the block start line. Since there is another cop that does that already, it isn't quite in the scope of this cop's description, and it would increase the complexity of the autocorrect code, I prefer not to add the logic here. Feel free to add a commit to include the functionality if you disagree, of course :).

bbatsov added a commit that referenced this pull request Jul 17, 2014
@bbatsov bbatsov merged commit 4bd4a72 into rubocop:master Jul 17, 2014
@bbatsov
Copy link
Collaborator

bbatsov commented Jul 17, 2014

👍

@jonas054
Copy link
Collaborator

Added some comments to the code. Sorry for being so late. I hope you can take a look soon, @barunio. There are some things that need to be fixed, I think.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 18, 2014

Guess I should have read the code more carefully.

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