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

[stdlib] Refactor and optimize String.replace() #3860

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

martinvuyk
Copy link
Contributor

Refactor and optimize String.replace()

CC: @JoeLoser another place where we need #3548

@martinvuyk martinvuyk requested a review from a team as a code owner December 11, 2024 14:35
@skongum02 skongum02 deleted the branch modular:main January 29, 2025 18:59
@skongum02 skongum02 closed this Jan 29, 2025
@skongum02 skongum02 reopened this Jan 29, 2025
@skongum02 skongum02 changed the base branch from nightly to main January 29, 2025 20:37
@martinvuyk
Copy link
Contributor Author

Hi @ConnorGray this is why I wanted to have better iterator ergonomics in #3700:

s = String("123")
for c in s:
    ...

now gives the warning: warning: Use 'str.chars()' or 'str.char_slices()' instead.
I strongly dislike this. It should be an option to transform the _StringSliceIter.as_chars() or something along those lines (e.g. having CharIter(iter("123"))). Because this absolutely kills the pythonic default which is simple and makes it feel like writing Rust.

@martinvuyk martinvuyk requested a review from a team as a code owner February 27, 2025 14:21
@gryznar
Copy link
Contributor

gryznar commented Feb 27, 2025

Keeping these deprecations (no matter if they are good or not) may allow team to merge changes, while reverting them is not so obvious

@martinvuyk
Copy link
Contributor Author

martinvuyk commented Feb 27, 2025

I did use __iter__() in the code for this PR. I will remove the un-deprecation in favor of using codepoint_slices()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants