Skip to content

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

Merged
dhruvmanila merged 4 commits intomainfrom
dhruv/char-wise-width
Jul 1, 2024
Merged

Use char-wise width instead of str-width#12135
dhruvmanila merged 4 commits intomainfrom
dhruv/char-wise-width

Conversation

@dhruvmanila
Copy link
Copy Markdown
Member

@dhruvmanila dhruvmanila commented Jul 1, 2024 •
edited
Loading

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 1, 2024 •
edited
Loading

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 force-pushed the dhruv/char-wise-width branch from 2e76825 to 0cd98a3 Compare July 1, 2024 12:01
@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
@dhruvmanila dhruvmanila requested a review from MichaReiser July 1, 2024 12:34
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jul 1, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #12135 will not alter performance

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

Summary

✅ 30 untouched benchmarks

MichaReiser
MichaReiser reviewed Jul 1, 2024
View reviewed changes
.../src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E501_E501_4.py.snap
Copy link
Copy Markdown
Member

@MichaReiser MichaReiser Jul 1, 2024

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
Copy Markdown
Member Author

@dhruvmanila dhruvmanila Jul 1, 2024

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
Copy Markdown
Member Author

@dhruvmanila dhruvmanila Jul 1, 2024

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

MichaReiser
MichaReiser reviewed Jul 1, 2024
View reviewed changes
@dhruvmanila dhruvmanila force-pushed the dhruv/char-wise-width branch from 70928fb to b49f0d6 Compare July 1, 2024 13:11
@dhruvmanila dhruvmanila merged commit 5677614 into main Jul 1, 2024
@dhruvmanila dhruvmanila deleted the dhruv/char-wise-width branch July 1, 2024 13:26
@dhruvmanila dhruvmanila mentioned this pull request Jul 1, 2024
Closed
@BrewTestBot BrewTestBot mentioned this pull request Jul 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Panic when checking file

2 participants

@dhruvmanila @MichaReiser