-
Notifications
You must be signed in to change notification settings - Fork 7
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
Document code style #209
Labels
meta
Abstract issues, for example administrative and procedural matters
Comments
tpitkanen
added
the
meta
Abstract issues, for example administrative and procedural matters
label
Oct 26, 2021
tpitkanen
added a commit
that referenced
this issue
Apr 7, 2022
Code style is now documented, but the line length part is still relevant, so I'm keeping this open. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Potku follows PEP 8, but this isn't currently documented anywhere. I think it should be documented in the readme, maybe under a Code style header.
Line length
I find the 80 (actually 79) character line length limit too restrictive. I think it should be increased to a soft limit of 100, and a hard limit of 120. Fewer split lines would take less horizontal space and make the code more readable. Using the
\
continuation character on several lines in a row is silly.Split strings are also problematic: they may not show up in a global text search if they happen to be split. Oftentimes the best way to find a UI component's corresponding code is with a text search.
The main issue with increasing the line length limit is merging: 3-way-merges don't fit on a 1920x1080 monitor unless the line length is roughly 80. I don't think this is that big of deal considering how little development time is spent on merge conflicts. Soft wrapping can be (temporarily) enabled for merges anyway.
If the line length limit is increased, the whole codebase should be updated at once. The change shouldn't affect functionality at all, so it should be easy to do.
The text was updated successfully, but these errors were encountered: