implement replace_with and replacen_with for unstable feature replace_with#147741
implement replace_with and replacen_with for unstable feature replace_with#147741alpaylan wants to merge 2 commits intorust-lang:mainfrom
replace_with and replacen_with for unstable feature replace_with#147741Conversation
rustbot
added
the
S-waiting-on-review
rustbot
added
the
T-libs
library/alloc/src/str.rs
Outdated
| if let Some(from_byte) = match from.as_utf8_pattern() { | ||
| Some(Utf8Pattern::StringPattern([from_byte])) => Some(*from_byte), | ||
| Some(Utf8Pattern::CharPattern(c)) => c.as_ascii().map(|ascii_char| ascii_char.to_u8()), | ||
| _ => None, | ||
| } { | ||
| if let Some(sref) = from_byte.as_ascii().map(|ascii_char| f(ascii_char.as_str())) { | ||
| if let [to_byte] = sref.as_ref().as_bytes() { | ||
| return unsafe { replace_ascii(self.as_bytes(), from_byte, *to_byte) }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Would it be possible to use let chains here?
There was a problem hiding this comment.
Updated with an if-let chain, did you have something else in mind or is this fine? (f70b5e1)
This comment has been minimized.
This comment has been minimized.
|
Hm, that's weird because alloc is 2024 edition Line 11 in f524236 |
|
It does compile with edition 2021 anyway tho Well, I guess that's fine to revert this then :) If this ever will get 2024 edition I will cleanup this accordingly |
library/alloc/src/str.rs
Outdated
| from: P, | ||
| mut f: impl FnMut(&str) -> S, | ||
| ) -> String { | ||
| // Fast path for replacing a single ASCII character with another. |
There was a problem hiding this comment.
f might not be side-effect-free, so this optimisation is incorrect.
There was a problem hiding this comment.
Ah, that is correct. I'm assuming I should just remove it, or are there any other potential optimization?
rustbot
added
S-waiting-on-author
| #[must_use = "this returns the replaced string as a new allocation, \ | ||
| without modifying the original"] | ||
| #[unstable(feature = "replace_with", issue = "147731")] | ||
| pub fn replacen_with<P: Pattern, S: AsRef<str>>( |
There was a problem hiding this comment.
weired name, easy to confuse with replace_with
we already have sothing alike repeat_n for iterator.
better with name replace_n_with to distinguish it.
saved a single character _ does not save much typing be hard to reading
There was a problem hiding this comment.
The name comes from an existing function, str::replacen, so the addition is the _with. That makes me think changing of replacen to replace_n should be a separate issue.
6 participants
This is the PR for the tracking issue #147731 for implementation of
replace_with.