-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Pyupgrade: Printf string formatting #1803
Pyupgrade: Printf string formatting #1803
Conversation
@charliermarsh I have hit a blocker and I would love some assistance. Pyupgrade uses the regex "(?<!\)(?:\\)*(\N{[^}]+})" to get an outcome like below: We cannot use this regex in rust, because the regex uses look behinds, which are not implemented in rust. I tried taking the post's advice and reimplementing the regex without look behinds, "^\*(\N{[^}]+})", but then the character before is included in the string, which causes the program to break: After this I tried using the If we do go the ignore/only report a check route I believe this is the regex we should use: "(?:\\)*(\N{[^}]+})" |
Why do we need / why does the regex exist? Is it to avoid treating named unicode characters as |
I know its for statements like these: |
It looks like it exists so that But we DO need to convert |
Is there a way that we could run the regex, extract the match, then run a second regex over the match to get a more precise range? |
Right now the regex I have developed that is the closest is
Additionally, I am not sure if we could use this "double regex" with methods like Basically all we need to confirm is that there is an odd number of |
Okay thanks. Just LMK when it's ready. I'll point to the updated RustPython branch and get it into a finished state, hopefully. |
And yeah -- I think we map |
@charliermarsh I am ready for you. Running |
Working my way through this one, found at least one bug but I think it might be in our logic and not the parser. |
@colin99d - Ok, I think this is passing all tests now. I can think of a few ways that it could be improved so I might iterate on it a bit more. (For example: it'd be nice if we could avoid transforms that create really long string format calls.) It's a really significant transform, so I want it to be rock-solid before it goes out. |
One thing I did in another check, is that if the new string is longer I would not do the change. But sounds good! Thanks for helping me out on this. It was definitely a lot trickier than I thought it would be. |
I guess I'm also thinking about whether we should be fixing multi-line strings. |
I can see it either way. Pyupgrade did NOT fix the strings that you sent me earlier that broke. |
Alright, I'm gonna merge this as soon as RustPython/RustPython#4463 is merged. |
[![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) | `^0.0.228` -> `^0.0.229` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.229/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.229/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.229/compatibility-slim/0.0.228)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.229/confidence-slim/0.0.228)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.229`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.229) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.228...v0.0.229) ##### What's Changed - README: `--force-exclude` is already set by [@​hugovk](https://togithub.com/hugovk) in [https://github.com/charliermarsh/ruff/pull/2042](https://togithub.com/charliermarsh/ruff/pull/2042) - Upgrade to toml v0.5.11 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/charliermarsh/ruff/pull/2040](https://togithub.com/charliermarsh/ruff/pull/2040) - Add support for pycodestyle E101 by [@​ericroberts](https://togithub.com/ericroberts) in [https://github.com/charliermarsh/ruff/pull/2038](https://togithub.com/charliermarsh/ruff/pull/2038) - \[`flake8-executable`] EXE003-005 by [@​sbrugman](https://togithub.com/sbrugman) in [https://github.com/charliermarsh/ruff/pull/2023](https://togithub.com/charliermarsh/ruff/pull/2023) - perf: Reduce allocations by [@​Stranger6667](https://togithub.com/Stranger6667) in [https://github.com/charliermarsh/ruff/pull/2045](https://togithub.com/charliermarsh/ruff/pull/2045) - refactor: RuleOrigin, RuleCodePrefix and Rule::origin by [@​not-my-profile](https://togithub.com/not-my-profile) in [https://github.com/charliermarsh/ruff/pull/2029](https://togithub.com/charliermarsh/ruff/pull/2029) - Add scaffolding for `flake8-type-checking` extension by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/charliermarsh/ruff/pull/2048](https://togithub.com/charliermarsh/ruff/pull/2048) - De-duplicate SIM102 by [@​harupy](https://togithub.com/harupy) in [https://github.com/charliermarsh/ruff/pull/2050](https://togithub.com/charliermarsh/ruff/pull/2050) - Fix S101 range to only highlight `assert` by [@​harupy](https://togithub.com/harupy) in [https://github.com/charliermarsh/ruff/pull/2052](https://togithub.com/charliermarsh/ruff/pull/2052) - Avoid removing comments in RUF005 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/charliermarsh/ruff/pull/2057](https://togithub.com/charliermarsh/ruff/pull/2057) - Revert "Upgrade to toml v0.5.11" by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/charliermarsh/ruff/pull/2058](https://togithub.com/charliermarsh/ruff/pull/2058) - Pyupgrade: Printf string formatting by [@​colin99d](https://togithub.com/colin99d) in [https://github.com/charliermarsh/ruff/pull/1803](https://togithub.com/charliermarsh/ruff/pull/1803) - \[`flake8-builtins`] Add `builtins-ignorelist` Option by [@​saadmk11](https://togithub.com/saadmk11) in [https://github.com/charliermarsh/ruff/pull/2061](https://togithub.com/charliermarsh/ruff/pull/2061) - feat: plugin scaffold for tryceratops with TRY300 by [@​sbrugman](https://togithub.com/sbrugman) in [https://github.com/charliermarsh/ruff/pull/2055](https://togithub.com/charliermarsh/ruff/pull/2055) - Avoid flagging redefined imports as unused in same-scope by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/charliermarsh/ruff/pull/2065](https://togithub.com/charliermarsh/ruff/pull/2065) ##### New Contributors - [@​hugovk](https://togithub.com/hugovk) made their first contribution in [https://github.com/charliermarsh/ruff/pull/2042](https://togithub.com/charliermarsh/ruff/pull/2042) - [@​ericroberts](https://togithub.com/ericroberts) made their first contribution in [https://github.com/charliermarsh/ruff/pull/2038](https://togithub.com/charliermarsh/ruff/pull/2038) - [@​sbrugman](https://togithub.com/sbrugman) made their first contribution in [https://github.com/charliermarsh/ruff/pull/2023](https://togithub.com/charliermarsh/ruff/pull/2023) **Full Changelog**: astral-sh/ruff@v0.0.228...v0.0.229 </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:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDguMSIsInVwZGF0ZWRJblZlciI6IjM0LjEwOC4xIn0=--> Signed-off-by: Renovate Bot <bot@renovateapp.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
A part of #827. This one is complicated so it will take me some more time.
Please Note: I purposely did NOT implement the
--keep-percent-format
flag because it looks like the last flag I added was removed. If you want this flag in, just let me know.Note: the issue of the /N{dog} format will plague us in UP030 and UP032. Whatever solution we find here needs to be applied in both of those places. Right now I am just thinking of looking for it, and if it exists, only showing a warning but not attempting top fix. I have NEVER seen this syntax in practice, and I think it is reasonable to give the people using it a warning, but don't fix.