-
Notifications
You must be signed in to change notification settings - Fork 750
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
Add configuration for blank lines. #732
Comments
PEP8 is pretty clear about the expectations w.r.t blank lines. And as @sigmavirus24 likes to say, This tool is firmly rooted in the documentation. That means it would run counter to the goals of pycodestyle to support this out of the box. However, in that same section is says this:
I tried it really quickly on my own, and pycodestyle doesn't actually support the extra lines to separate groups of related functions. Personally, I don't do that, but it is a bit of a departure from PEP8. And given my (admittedly limited) knowledge of how pycodestyle works, I imagine it would be difficult to allow extra blank lines 'sparingly'. So I think the real question to the community here is: Do we prefer strict 2/1 lines between things, or do we want to allow 3/2 (with the caveat that it tacitly consents to putting 3 and 2 lines between everything) (of course, if we could define 'sparingly' in a way that works for the vast majority of use cases, maybe we could have this be properly enforced. I have no idea how to do that without spawning a pitchfork-wielding mob :p) @adiroiban In the meantime, I'm curious why your team wants to use 3 and 2 lines between top-level and class members. I'd find that quite distracting myself, and a bit of a waste of screen space as well, so I'm interested in what benefit you derive. |
@hoylemd Thanks for looking into this. As a start, I would be happy if the logic of top blank lines vs method blank lines would be contained into some variables which are easy to overwrite ... like in this PR. I am not expecting for something out of the box. I think that this PR, as it is now, can help the pycodestyle project as it extracts the main variables from all the conditionals in the blank_lines function. Now you have a comparison with 2, but you don't if that is for top_level or for method blank lines as 1 + 1 What I wanted to do, is to stop using a fork of pycodestyle for Twisted :) I don't know which version is best or why Twisted went with this convention. I understand that extra blank lines are a waste of space.
I find 1 blank lines between methods districting as there are also 1 blank lines inside the body of the method... so is harder to tell where the method ends :) |
I think the propsed changes in #733 are perfectly fine, with a tad bit of work. I think that if twisted wants to add configuration options, however, to control these, those belong as a plugin for |
I am fine to not have the configuration in the pycodestyle command line. |
This adds some module level configuration points for users to define how many blank lines they want in their code. It paves the way for someone to develop a flake8 plugin to configure this in pycodestyle. Fixes #732
I know that a lot of Python code is using a single blank line for methods and 2 blank lines for classes.
Still, I would like to see a flag, to switch to 2 blank lines for methods and 3 blank lines for classes.
In this way, you can use 1 blank line inside a method and don't confuse it with a method delimiter.
This style is used by the Twisted project and for a couple of years Twisted was using a fork which changes the blank_lines function in pycodestyle.
I am starting this discussion to see whether pycodestyle maintainer are happy/willing to help with having support for this upstream, so that we don't have to keep the fork :)
Here is a patch, which extracts the blank lines values into local variables (for now) master...adiroiban:configurable-blank-lines
If such a functionality is accepted, I am happy to write the implementation and tests.
Thanks a lot for your work. Pycodestyle is of great help in keeping Twisted code clean :)
The text was updated successfully, but these errors were encountered: