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

Possibility to implement flake8-length into ruff #4595

Open
jsh9 opened this issue May 23, 2023 · 9 comments
Open

Possibility to implement flake8-length into ruff #4595

jsh9 opened this issue May 23, 2023 · 9 comments
Labels
needs-decision Awaiting a decision from a maintainer plugin Implementing a known but unsupported plugin

Comments

@jsh9
Copy link

jsh9 commented May 23, 2023

Hi,

I've found that flake8-length written by @orsinium to be one of my favorite flake8 plugins. I searched the issue pool of ruff and didn't find anyone mentioning it.

Thank you!

@orsinium
Copy link

image

@charliermarsh charliermarsh added the plugin Implementing a known but unsupported plugin label May 24, 2023
@charliermarsh
Copy link
Member

I think my preference would be to modify our existing rule rather than add multiple line-length rules within Ruff itself. Same goes for flake8-bugbear's variant that allows a 10% buffer. E.g., we do allow long lines that end in URLs, as long as the URL starts before the length limit (which differs from pycodestyle). We also allow lines that consistent of a single token without whitespace to break it up.

What other behaviors or exceptions might you find helpful in our check?

@charliermarsh charliermarsh added the question Asking for support or clarification label May 24, 2023
@jsh9
Copy link
Author

jsh9 commented May 24, 2023

Hi Charlie,

Ideally for me, an "intelligent" line length checker should ignore these situations:

  • Long string literals (if the string starts before the length limit)
  • Comments (if the non-comment part of the line ends before the length limit)

flake8-length can do both, which is why I like it. Also, the violation code of flake8-length is LN, which is intuitive (better than E501).

@orsinium
Copy link

What other behaviors or exceptions might you find helpful in our check?

Flake8-length allows:

  • Long string literals.
  • Long URLs in strings and comments.
  • When the last word in a text (comment, string, or docstring) doesn't fit a bit.
  • Long shebangs.
  • noqa:, pragma: and similar comments that should appear on the same line as their violation and so cannot be easily moved or split.

The motivation is described in the README, but in short, these are things you want to keep unbroken for the sake of grep'ability.

It also allows certain symbols and tokens to appear after the string. So, a: f('very long text'), won't be reported () and , are allowed to go after) but f('very long text').something_else() will be reported because the something_else part goes beyond the screen but it's significant for the reader to see.

There is also a specific exception for raw SQL queries. SQL allows newlines between tokens, so they can be safely reformatted into a multiline string literal.

I guess that's it. The parser's source code is just about 100 lines.

@charliermarsh charliermarsh added needs-decision Awaiting a decision from a maintainer and removed question Asking for support or clarification labels Jul 10, 2023
@stinodego
Copy link
Contributor

I have tried flake8-length before, but I found that I get pretty much the same behavior by ignoring E501 (line too long) and enabling pycodestyle's max-doc-length:


[tool.ruff]
line-length = 88

select = [
  "E", # pycodestyle
  "W", # pycodestyle
]
ignore = [
  "E501",  # Line length regulated by black
]

[tool.ruff.pycodestyle]
max-doc-length = 88

@orsinium
Copy link

but I found that I get pretty much the same behavior by ignoring E501 (line too long) <...>

The point of flake8-length is to not remove the line length limit for the code. From the README:

If you're about having as strict limits as possible, flake8-length is on your side. It's better to set 90 chars limit with a few reasonable exceptions rather than have 120 or more chars limit for everything.

@stinodego
Copy link
Contributor

Yes, the point being that black already takes care of that!

It will wrap anything except for very long strings and comments. And the max-doc-length setting will then warn you about certain long strings / comments, but has reasonable exceptions built-in. So it works out to be approximately the same (in my experience) as what the flake8-length plugin is trying to achieve.

If you take black out of the picture, then it becomes a whole different conversation.

@lev-blit
Copy link

@stinodego I agree that black takes care of that, but it adds a large overhead. There's a project I worked on which had about 200~ files, with some of them being very large, and running ruff was shorter than a blink of an eye but running black against all those files felt like an eternity compared to that (and it was quite long without comparison too - about 8 seconds). That's just because ruff doesn't ignore comments and other examples given above regarding flake8-length.

It would be really useful for ruff to be able to support those features, then I would be able to use black only as a formatting tool and not as a linting tool just for E501.

@RosanneZe
Copy link

We're looking to switch from flake8 to ruff, but we would like to keep the behaviour that flake8-length gives. So I'd like to add my vote to adding it as a plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer plugin Implementing a known but unsupported plugin
Projects
None yet
Development

No branches or pull requests

6 participants