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

Allow configuration of trailing blank lines #490

Closed

Conversation

daviddavis
Copy link
Contributor

No description provided.

end
end

def max
cop_config['Max'] || 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should add the Max parameter in config/default.yml. Then you can remove || 0 because all configurations are based on the default so cop_config['Max'] is guaranteed to have a non-nil value.

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 17, 2013

Not sure such an option makes sense. Why would anyone want to have some trailing blank lines in their source code?

@daviddavis
Copy link
Contributor Author

In our project, a lot of our files end with 1 trailing blank line:

class Whatever
# ...
end

I figured we would allow 1 trailing blank line. I'm leaning toward just cleaning up our files though so I will close this out.

@daviddavis daviddavis closed this Sep 17, 2013
@bbatsov
Copy link
Collaborator

bbatsov commented Sep 17, 2013

By common convention (not only for Ruby) source files should end with just a newline. It doesn't make sense for me to start breaking such strongly established practice.

@jdickey
Copy link

jdickey commented Sep 27, 2013

There's a big difference between "newline at end of file" and "blank line at end of file" (two consecutive newlines). Some editors (e.g. Komodo) automatically add a blank line if one does not exist, as many people do find it easier to read that way. No team that I've been on in the last three decades used a standard requiring no blank lines at the end of files. Granted, Ruby has only been involved in the last ~5 years of that, but springing this new behaviour on people and having every single file report a violation is a bit much. I see the value in warning of multiple blank lines at the ends of files. I see no value whatever in a change that requires that an otherwise-useful new feature (the cop under discussion here) be disabled to avoid requiring touching every single source file in a project.

I know cops aren't doctors, but "first, do no harm" really should apply here, too.

@aaronmcadam
Copy link

Why was this not merged? How can I turn off this cop?

@jonas054
Copy link
Collaborator

The author closed it so it would have to be reopened first in order to be merged. I'm not sold on the idea of a Max parameter, but a parameter acting like a boolean switch, e.g. EnforcedStyle with values no_line (default) and one_line would be an improvement IMO, especially in light of what @jdickey said.

@bbatsov If you agree I can implement that and make a new pull request.

@aaronmcadam To disable the cop, just add

TrailingBlankLines:
  Enabled: false

in your .rubocop.yml file.

@aaronmcadam
Copy link

@jonas054 Yeah thanks I figured that bit out! What I'd like to figure out now is how to stop the autocorrect directive from removing the trailing new line when it runs.

@jonas054
Copy link
Collaborator

If you've disabled the cop, there will be no auto-correct action related to that offence.

@aaronmcadam
Copy link

@jonas054 ah, my bad, just tried it again and it's ok, must have been something else! Thanks for helping :)

@daviddavis
Copy link
Contributor Author

@jonas054 I'd be happy to work on an EnforcedStyle config unless you want to.

@jonas054
Copy link
Collaborator

@daviddavis It's yours then! 😄

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 16, 2014

@daviddavis @jonas054 As I originally said - I'm against such a change, but I can live with it. Let's have an enforced style param with supported values final_newline and final_blank_line.

@daviddavis
Copy link
Contributor Author

@bbatsov @jonas054 cool! thanks!

@jarinudom
Copy link

@bbatsov I would like to see this too, as there are a few reasons to have a trailing newline in your code. For example:

  • cat will put your prompt on the same line as the last line of output unless you have a trailing newline (not to mention that if you actually wanted to concatenate two files it would jam the lines together)
  • echo 'some_file' >> .gitignore won't work unless your .gitignore ends with a newline
  • grep expects a newline at the end of the file
  • Probably most importantly to me, diff and git changelogs will consider the (formerly last) line to have been changed when you add additional lines to the file, because you're adding a newline at that time. This is why both git and GitHub complain when you don't have newlines at the end of your files.

Couple of discussions here:
http://stackoverflow.com/questions/729692/why-should-files-end-with-a-newline
http://stackoverflow.com/questions/16072822/why-does-ruby-on-rails-generate-add-a-blank-line-to-the-end-of-a-file/16072987#16072987

I hope this doesn't come across as bikeshedding, I just wanted to point out that there are a bunch of practical reasons for it.

@jonas054 jonas054 mentioned this pull request Apr 9, 2014
@fabiopelosin
Copy link
Contributor

👍 for the EnforcedStyle

@jonas054
Copy link
Collaborator

jonas054 commented Apr 9, 2014

I'm reopening and assigning myself.

@jonas054 jonas054 reopened this Apr 9, 2014
@jonas054 jonas054 self-assigned this Apr 9, 2014
@daviddavis
Copy link
Contributor Author

@jonas054 thanks. I apologize for not getting around to it.

@jonas054
Copy link
Collaborator

@daviddavis No problem!

@jonas054 jonas054 closed this in 4342d30 Apr 11, 2014
@fabiopelosin
Copy link
Contributor

Nice! 🍻

@jarinudom
Copy link

Yay thank you :)

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.

7 participants