-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Add str::strip_prefix and str::strip_suffix #66735
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Ping from triage, any updates? @withoutboats |
r? @KodrAus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming wise it looks like we've got a few schemes going here: trim
vs strip
and start
/end
vs prefix
/suffix
.
Personally, I think strip_prefix
and strip_suffix
are nicely discoverable names, and this API is distinct enough from the trim
ones in the way it operates that it doesn't seem necessary to me to make them consistent. They can sit out as distinct so users don't have to try compare signatures to see what makes them different.
So this looks good to me! I'll open up a tracking issue and circle back, but r=me if anybody beats me to it 🙂
@KodrAus can I merge this? |
Ping from triage - @KodrAus , see Dylan-DPC's comment |
This still needs the tracking issue created and to be linked up to, then r=@KodrAus. |
Yes. But I don't want to keep the Pr hanging on an issue creation which can happen at a later stage as well |
📌 Commit 4718e20 has been approved by |
The PR can't land before tracking issue creation because the issue number needs to go inside the #[unstable(...)] attribute. We shouldn't be merging unstable methods with @bors r- |
Ah right, my bad 😆 |
Whoops! Looks like I preempted my own dropping of the ball here. Thanks for opening the tracking issue @Dylan-DPC. @bors r+ |
📌 Commit 6176051 has been approved by |
🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened |
Add str::strip_prefix and str::strip_suffix Introduces a counterpart for `Path::strip_prefix` on `str`. This was also discussed in https://internals.rust-lang.org/t/pre-pr-path-strip-prefix-counterpart-in-str/11364/.
Rollup of 7 pull requests Successful merges: - #65778 (Stabilize `std::{rc,sync}::Weak::{weak_count, strong_count}`) - #66570 (stabilize Result::map_or) - #66735 (Add str::strip_prefix and str::strip_suffix) - #66771 (Stabilize the `core::panic` module) - #67317 (fix type_name_of_val doc comment) - #67324 (Fix repetition in matches/mod.rs) - #67325 (cleanup with push_fake_read) Failed merges: r? @ghost
Rollup of 7 pull requests Successful merges: - #65778 (Stabilize `std::{rc,sync}::Weak::{weak_count, strong_count}`) - #66570 (stabilize Result::map_or) - #66735 (Add str::strip_prefix and str::strip_suffix) - #66771 (Stabilize the `core::panic` module) - #67317 (fix type_name_of_val doc comment) - #67324 (Fix repetition in matches/mod.rs) - #67325 (cleanup with push_fake_read) Failed merges: r? @ghost
Introduces a counterpart for
Path::strip_prefix
onstr
.This was also discussed in https://internals.rust-lang.org/t/pre-pr-path-strip-prefix-counterpart-in-str/11364/.