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.

Sign up for GitHub

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

Jump to bottom

Fixes an issue with empty lines around modifier keyword and beginning of a block #1553

Merged
merged 1 commit into from
Jan 8, 2015
Merged

Fixes an issue with empty lines around modifier keyword and beginning of a block #1553

merged 1 commit into from
Jan 8, 2015

Conversation

volkert
Copy link

@volkert volkert commented Jan 7, 2015

There is no way that the following code is valid for rubocop. It will either complain about the missing empty line above the access modifier (Style/EmptyLinesAroundAccessModifier) or the empty line at the beginning of a block (Style/EmptyLinesAroundBlockBody).

included do
  private

  def bar
  end
end

This example occured when using rails concerns.
This PR fixes this issue. Please let me know if I can improve my PR, I'm looking forward to contribute!

@volkert volkert force-pushed the empty-lines-with-blocks-and-modifiers branch 2 times, most recently from 8b43f72 to d4226fe Compare January 7, 2015 09:11
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 8b43f7271742b698b46f860625627e43da7b3689 on volkert:empty-lines-with-blocks-and-modifiers into 7650116 on bbatsov:master.

@volkert volkert force-pushed the empty-lines-with-blocks-and-modifiers branch from d4226fe to 7650116 Compare January 7, 2015 09:18
@volkert
Copy link
Author

volkert commented Jan 7, 2015

I don't know why the build on travis is failing, it doesn't seem to be related to my change. Am I missing something there?

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 7, 2015

Seems like some JRuby bug...

bbatsov
bbatsov reviewed Jan 7, 2015
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_access_modifier.rb
@@ -61,6 +63,10 @@ def class_def?(line)
%w(class module).any? { |keyword| line.start_with?(keyword) }
end

def block_start?(line)
line.match(/ do$/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't be robust enough, as someone might be using { for block delim.

@volkert volkert force-pushed the empty-lines-with-blocks-and-modifiers branch from 19b8ec8 to 86f1e6c Compare January 7, 2015 16:55
@coveralls
Copy link

Coverage Status

Coverage increased (+0.32%) when pulling 86f1e6c2773b707cdcec87b53caef40012de3b3a on volkert:empty-lines-with-blocks-and-modifiers into 7fa6456 on bbatsov:master.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 7, 2015

The code seems OK to me now. Just squash the two commits together and we're good to go.

@volkert volkert force-pushed the empty-lines-with-blocks-and-modifiers branch from 86f1e6c to 90f383a Compare January 8, 2015 08:34
@volkert
Copy link
Author

volkert commented Jan 8, 2015

Done!

bbatsov
bbatsov reviewed Jan 8, 2015
View reviewed changes
spec/rubocop/cop/style/empty_lines_around_access_modifier_spec.rb
expect(cop.offenses).to be_empty
end

it 'accepts missing blank line with do and arguments' do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Guess we should have a spec like this for {} blocks as well.

@volkert volkert force-pushed the empty-lines-with-blocks-and-modifiers branch from 90f383a to d72e547 Compare January 8, 2015 09:15
@volkert volkert force-pushed the empty-lines-with-blocks-and-modifiers branch from d72e547 to dcecd86 Compare January 8, 2015 09:16
@volkert
Copy link
Author

volkert commented Jan 8, 2015

Like this?

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 8, 2015

👍 Indeed. Thanks for fixing this!

bbatsov added a commit that referenced this pull request Jan 8, 2015
@bbatsov
Merge pull request #1553 from volkert/empty-lines-with-blocks-and-mod…
f19b083 
…ifiers

Fixes an issue with empty lines around modifier keyword and beginning of a block
@bbatsov bbatsov merged commit f19b083 into rubocop:master Jan 8, 2015
@volkert volkert deleted the empty-lines-with-blocks-and-modifiers branch January 8, 2015 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@volkert @coveralls @bbatsov