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
Introduce tab-size
to correcly calculate the line length with tabulations
#4167
Introduce tab-size
to correcly calculate the line length with tabulations
#4167
Conversation
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinux
Windows
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
There are a few places where we do not suggest a code fix if the line length after fixing exceeds the limit. We need to change these checks to handle tabs correctly too.
https://github.com/search?q=repo%3Acharliermarsh%2Fruff%20.width()&type=code
7a748d4
to
35aec34
Compare
c3d9237
to
d908ea4
Compare
Should I add a fixture for all affected rules with tabulations and UTF-8 characters? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I add a fixture for all affected rules with tabulations and UTF-8 characters?
That would be awesome to ensure we don't accidentally break it in future refactors.
crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__tab_size_8.snap
Outdated
Show resolved
Hide resolved
d908ea4
to
b6f0c64
Compare
cda81f3
to
02cab1e
Compare
8ef580a
to
a892ffd
Compare
All concerned fixtures now have tests with UTF-8 and check with line length. Some also contain tabulations. |
a892ffd
to
bfe75b9
Compare
tab-size
to correcly calculate the line length with tabulations
bfe75b9
to
191b217
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I have a few comments around the implementations of the new width structs, and are uncertain if the caret is correctly placed in one test. But it is looking great overall!
crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__tab_size_1.snap
Show resolved
Hide resolved
impl Default for LineWidth<Length> { | ||
fn default() -> Self { | ||
Self { | ||
width: 88, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason that we initialize the width to 88
? Or what's the meaning of the width
field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LineWidth<Length>
or LineLength
is the type representing line length settings.
88
comes from LINE_LENGTH
.
117d307
to
451d494
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. I like it a lot how you centralized the logic for all the line width checks.
Could you add some documentation to LineLength
and LineWdith
and move LineWidth
out of the settings
module (it's not used in a settings context if I understand it correctly)?
451d494
to
ad8800d
Compare
Thank you. |
You are welcome, thank you for your review. |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://togithub.com/charliermarsh/ruff) ([changelog](https://togithub.com/charliermarsh/ruff/releases)) | `^0.0.269` -> `^0.0.270` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/compatibility-slim/0.0.269)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/confidence-slim/0.0.269)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.270`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.270) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.269...v0.0.270) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### Rules - \[`flake8-bandit`] Implement `paramiko-call` (`S601`) by [@​scop](https://togithub.com/scop) in [astral-sh/ruff#4500 - \[`flake8-pyi`] Add autofix for PYI009 by [@​qdegraaf](https://togithub.com/qdegraaf) in [astral-sh/ruff#4583 - \[`flake8-pyi`] Implement `PYI013` by [@​density](https://togithub.com/density) in [astral-sh/ruff#4517 - \[`pylint`] Add `duplicate-value` (`W0130`) by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4515 - \[`pylint`] Add `named_expr_without_context` (`W0131`) by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4531 - \[`ruff`] Extend `RUF005` to recursive and literal-literal concatenations by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4557 - \[`ruff`] Make ambiguous-unicode detection sensitive to 'word' context by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4552 - \[`ruff`] Name ambiguous characters by [@​covracer](https://togithub.com/covracer) in [astral-sh/ruff#4448 ##### Settings - Implement `--extend-fixable` option by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4297 - Support new `extend-per-file-ignores` setting by [@​aacunningham](https://togithub.com/aacunningham) in [astral-sh/ruff#4265 ##### Bug Fixes - Fix RUF010 auto-fix with parenthesis by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4524 - Parenthesize more sub-expressions in f-string conversion by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4535 - Fix false-positive for TRY302 if exception cause is given by [@​153957](https://togithub.com/153957) in [astral-sh/ruff#4559 - Fix `# isort: split` comment detection in nested blocks by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4584 - Avoid some false positives in dunder variable assigments by [@​scop](https://togithub.com/scop) in [astral-sh/ruff#4508 - Fix UP032 auto-fix with integers by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4525 - Improve reference resolution for deferred-annotations-within-classes by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4509 - Improve handling of `__qualname__`, `__module__`, and `__class__` by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4512 - Include empty success test in JUnit output by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4537 - Fix SIM110 and SIM111 ranges by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4545 - Ignore `#region` code folding marks in eradicate rules by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4546 - Avoid infinite loop for required imports with isort: off by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4581 - Make B007 fix relevance stricter by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4607 - Introduce `tab-size` to correcly calculate the line length with tabulations by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4167 - Visit `TypeVar` and `NewType` name arguments by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4627 - Improve `Message` sorting performance by [@​MichaReiser](https://togithub.com/MichaReiser) in [astral-sh/ruff#4624 #### New Contributors - [@​hoel-bagard](https://togithub.com/hoel-bagard) made their first contribution in [astral-sh/ruff#4516 - [@​density](https://togithub.com/density) made their first contribution in [astral-sh/ruff#4517 - [@​Mr-Pepe](https://togithub.com/Mr-Pepe) made their first contribution in [astral-sh/ruff#4540 - [@​153957](https://togithub.com/153957) made their first contribution in [astral-sh/ruff#4559 - [@​covracer](https://togithub.com/covracer) made their first contribution in [astral-sh/ruff#4448 **Full Changelog**: astral-sh/ruff@v0.0.269...v0.0.270 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/ixm-one/pytest-cmake-presets). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS45OC40IiwidXBkYXRlZEluVmVyIjoiMzUuOTguNCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Signed-off-by: Renovate Bot <bot@renovateapp.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://togithub.com/charliermarsh/ruff) ([changelog](https://togithub.com/charliermarsh/ruff/releases)) | `==0.0.269` -> `==0.0.270` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/compatibility-slim/0.0.269)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/confidence-slim/0.0.269)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.270`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.270) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.269...v0.0.270) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### Rules - \[`flake8-bandit`] Implement `paramiko-call` (`S601`) by [@​scop](https://togithub.com/scop) in [astral-sh/ruff#4500 - \[`flake8-pyi`] Add autofix for PYI009 by [@​qdegraaf](https://togithub.com/qdegraaf) in [astral-sh/ruff#4583 - \[`flake8-pyi`] Implement `PYI013` by [@​density](https://togithub.com/density) in [astral-sh/ruff#4517 - \[`pylint`] Add `duplicate-value` (`W0130`) by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4515 - \[`pylint`] Add `named_expr_without_context` (`W0131`) by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4531 - \[`ruff`] Extend `RUF005` to recursive and literal-literal concatenations by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4557 - \[`ruff`] Make ambiguous-unicode detection sensitive to 'word' context by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4552 - \[`ruff`] Name ambiguous characters by [@​covracer](https://togithub.com/covracer) in [astral-sh/ruff#4448 ##### Settings - Implement `--extend-fixable` option by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4297 - Support new `extend-per-file-ignores` setting by [@​aacunningham](https://togithub.com/aacunningham) in [astral-sh/ruff#4265 ##### Bug Fixes - Fix RUF010 auto-fix with parenthesis by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4524 - Parenthesize more sub-expressions in f-string conversion by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4535 - Fix false-positive for TRY302 if exception cause is given by [@​153957](https://togithub.com/153957) in [astral-sh/ruff#4559 - Fix `# isort: split` comment detection in nested blocks by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4584 - Avoid some false positives in dunder variable assigments by [@​scop](https://togithub.com/scop) in [astral-sh/ruff#4508 - Fix UP032 auto-fix with integers by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4525 - Improve reference resolution for deferred-annotations-within-classes by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4509 - Improve handling of `__qualname__`, `__module__`, and `__class__` by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4512 - Include empty success test in JUnit output by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4537 - Fix SIM110 and SIM111 ranges by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4545 - Ignore `#region` code folding marks in eradicate rules by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4546 - Avoid infinite loop for required imports with isort: off by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4581 - Make B007 fix relevance stricter by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4607 - Introduce `tab-size` to correcly calculate the line length with tabulations by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4167 - Visit `TypeVar` and `NewType` name arguments by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4627 - Improve `Message` sorting performance by [@​MichaReiser](https://togithub.com/MichaReiser) in [astral-sh/ruff#4624 #### New Contributors - [@​hoel-bagard](https://togithub.com/hoel-bagard) made their first contribution in [astral-sh/ruff#4516 - [@​density](https://togithub.com/density) made their first contribution in [astral-sh/ruff#4517 - [@​Mr-Pepe](https://togithub.com/Mr-Pepe) made their first contribution in [astral-sh/ruff#4540 - [@​153957](https://togithub.com/153957) made their first contribution in [astral-sh/ruff#4559 - [@​covracer](https://togithub.com/covracer) made their first contribution in [astral-sh/ruff#4448 **Full Changelog**: astral-sh/ruff@v0.0.269...v0.0.270 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/allenporter/pyrainbird). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMDIuNCIsInVwZGF0ZWRJblZlciI6IjM1LjEwMi40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://togithub.com/charliermarsh/ruff) ([changelog](https://togithub.com/charliermarsh/ruff/releases)) | `==0.0.269` -> `==0.0.270` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/compatibility-slim/0.0.269)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.270/confidence-slim/0.0.269)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.270`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.270) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.269...v0.0.270) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### Rules - \[`flake8-bandit`] Implement `paramiko-call` (`S601`) by [@​scop](https://togithub.com/scop) in [astral-sh/ruff#4500 - \[`flake8-pyi`] Add autofix for PYI009 by [@​qdegraaf](https://togithub.com/qdegraaf) in [astral-sh/ruff#4583 - \[`flake8-pyi`] Implement `PYI013` by [@​density](https://togithub.com/density) in [astral-sh/ruff#4517 - \[`pylint`] Add `duplicate-value` (`W0130`) by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4515 - \[`pylint`] Add `named_expr_without_context` (`W0131`) by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4531 - \[`ruff`] Extend `RUF005` to recursive and literal-literal concatenations by [@​hoel-bagard](https://togithub.com/hoel-bagard) in [astral-sh/ruff#4557 - \[`ruff`] Make ambiguous-unicode detection sensitive to 'word' context by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4552 - \[`ruff`] Name ambiguous characters by [@​covracer](https://togithub.com/covracer) in [astral-sh/ruff#4448 ##### Settings - Implement `--extend-fixable` option by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4297 - Support new `extend-per-file-ignores` setting by [@​aacunningham](https://togithub.com/aacunningham) in [astral-sh/ruff#4265 ##### Bug Fixes - Fix RUF010 auto-fix with parenthesis by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4524 - Parenthesize more sub-expressions in f-string conversion by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4535 - Fix false-positive for TRY302 if exception cause is given by [@​153957](https://togithub.com/153957) in [astral-sh/ruff#4559 - Fix `# isort: split` comment detection in nested blocks by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4584 - Avoid some false positives in dunder variable assigments by [@​scop](https://togithub.com/scop) in [astral-sh/ruff#4508 - Fix UP032 auto-fix with integers by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4525 - Improve reference resolution for deferred-annotations-within-classes by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4509 - Improve handling of `__qualname__`, `__module__`, and `__class__` by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4512 - Include empty success test in JUnit output by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4537 - Fix SIM110 and SIM111 ranges by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4545 - Ignore `#region` code folding marks in eradicate rules by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4546 - Avoid infinite loop for required imports with isort: off by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4581 - Make B007 fix relevance stricter by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4607 - Introduce `tab-size` to correcly calculate the line length with tabulations by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#4167 - Visit `TypeVar` and `NewType` name arguments by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#4627 - Improve `Message` sorting performance by [@​MichaReiser](https://togithub.com/MichaReiser) in [astral-sh/ruff#4624 #### New Contributors - [@​hoel-bagard](https://togithub.com/hoel-bagard) made their first contribution in [astral-sh/ruff#4516 - [@​density](https://togithub.com/density) made their first contribution in [astral-sh/ruff#4517 - [@​Mr-Pepe](https://togithub.com/Mr-Pepe) made their first contribution in [astral-sh/ruff#4540 - [@​153957](https://togithub.com/153957) made their first contribution in [astral-sh/ruff#4559 - [@​covracer](https://togithub.com/covracer) made their first contribution in [astral-sh/ruff#4448 **Full Changelog**: astral-sh/ruff@v0.0.269...v0.0.270 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/allenporter/flux-local). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMDIuMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNS4xMDIuMTAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
I added the setting
tab-size
to set the tabulation width in checks likeE501
.I use as default
4
as it is the most common case.With
tab-size = 1
ortab-size = 2
:With
tab-size = 4
:With
tab-size = 8
: