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

Implement std::str::replacen #36347

Merged
merged 2 commits into from
Sep 15, 2016
Merged

Implement std::str::replacen #36347

merged 2 commits into from
Sep 15, 2016

Conversation

knight42
Copy link
Contributor

@knight42 knight42 commented Sep 8, 2016

Replaces first N matches of a pattern with another string.

assert_eq!("acaaa".replacen(a, "b", 3), "bcbba")

@knight42
Copy link
Contributor Author

knight42 commented Sep 8, 2016

It's rare that @rust-highfive didn't show up 😮

/// let s = "this is old";
/// assert_eq!(s, s.replacen("cookie monster", "little lamb", 10));
/// ```
#[stable(feature = "str_replacen", since = "1.13.0")]
Copy link
Member

@nagisa nagisa Sep 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insta-stable? AFAICT there’s no tracking issue which went through FCP for stabilisation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nagisa I'm sorry... Should I submit a RFC about this? BTW, where can I know the process of accepting a new method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An unstable method with a corresponding tracking issue may be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nagisa Am I supposed to open the tracking issue by myself?

Copy link
Member

@nagisa nagisa Sep 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d suggest to only do that after you get thumbs-up from the libs team for the feature you’re adding. In the mean-time you can use 0 as a placeholder issue for the attribute.

@nagisa
Copy link
Member

nagisa commented Sep 8, 2016

This PR needs a reviewer, @rust-lang/libs.

@knight42
Copy link
Contributor Author

knight42 commented Sep 8, 2016

@nagisa Thanks for your advice! 😄

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Sep 8, 2016

Seems quite niche to me.

let mut result = String::with_capacity(32);
let matched = self.match_indices(pat);
let mut last_end = 0;
for (_, (start, part)) in (0..count).zip(matched) {
Copy link
Member

@bluss bluss Sep 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using .take(count) would be the more typical way to limit the number of iterations here. (I'd understand it right away :D)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 8, 2016
@alexcrichton
Copy link
Member

Discussed at libs triage today the decision was to merge. @knight42 could you open an issue on rust-lang/rust to track the stabilization of this and update the #[unstable] tag reference? I'll be sure to tag it correctly once it's opened.

@knight42
Copy link
Contributor Author

@alexcrichton Great to hear that! 🎉

The #[unstable] tag has been updated now.

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Sep 13, 2016

📌 Commit ebda770 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Sep 14, 2016

⌛ Testing commit ebda770 with merge e25e32c...

@bors
Copy link
Contributor

bors commented Sep 14, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@knight42
Copy link
Contributor Author

@alexcrichton the failure seems spurious

@alexcrichton
Copy link
Member

@bors: retry

On Tue, Sep 13, 2016 at 8:59 PM, Jian Zeng notifications@github.com wrote:

@alexcrichton https://github.com/alexcrichton the failure seems
spurious


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36347 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95MD5gYcTCvYvO5_d1JvbEp2EOplhks5qp3EegaJpZM4J33bo
.

@bors
Copy link
Contributor

bors commented Sep 15, 2016

⌛ Testing commit ebda770 with merge 16ff9e2...

bors added a commit that referenced this pull request Sep 15, 2016
Implement std::str::replacen

Replaces first N matches of a pattern with another string.

```
assert_eq!("acaaa".replacen(a, "b", 3), "bcbba")
```
@bors bors merged commit ebda770 into rust-lang:master Sep 15, 2016
@knight42 knight42 deleted the str-replacen branch September 15, 2016 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants