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

Use char-wise width instead of str-width #12135

Merged
merged 4 commits into from
Jul 1, 2024
Merged

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jul 1, 2024

Summary

This PR updates various references in the linter to compute the line-width for summing the width of each char in a str instead of computing the width of the str itself.

Refer to #12133 for more details.

fixes: #12130

Test Plan

Add a file with null (\0) character which is zero-width. Run this test case on main to make sure it panics and switch over to this branch to make sure it doesn't panic now.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check encountered linter errors. (no lint changes; 1 project error)

demisto/content (error)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'unfixable' -> 'lint.unfixable'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
warning: `PGH001` has been remapped to `S307`.
warning: `PGH002` has been remapped to `G010`.
warning: `PLR1701` has been remapped to `SIM101`.
ruff failed
  Cause: Selection of deprecated rule `E999` is not allowed when preview is enabled.

@dhruvmanila dhruvmanila added the bug Something isn't working label Jul 1, 2024
@dhruvmanila dhruvmanila marked this pull request as ready for review July 1, 2024 12:33
Copy link

codspeed-hq bot commented Jul 1, 2024

CodSpeed Performance Report

Merging #12135 will not alter performance

Comparing dhruv/char-wise-width (a8cd872) with main (37f260b)

Summary

✅ 30 untouched benchmarks

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, why is this considered to be a binary file?

Copy link
Member Author

Choose a reason for hiding this comment

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

It contains \0 character. I think I'll just inline the source code of that file as a separate test case to not do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's just the diff tab that renders it like that. The normal GitHub view is as: https://github.com/astral-sh/ruff/blob/dhruv/char-wise-width/crates/ruff_linter/resources/test/fixtures/pycodestyle/E501_4.py

@dhruvmanila dhruvmanila merged commit 5677614 into main Jul 1, 2024
20 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/char-wise-width branch July 1, 2024 13:26
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when checking file
2 participants