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

Add method str::repeat(self, usize) -> String #36699

Merged
merged 1 commit into from
Oct 12, 2016
Merged

Conversation

bluss
Copy link
Member

@bluss bluss commented Sep 24, 2016

It is relatively simple to repeat a string n times:
(0..n).map(|_| s).collect::<String>(). It becomes slightly more
complicated to do it “right” (sizing the allocation up front), which
warrants a method that does it for us.

This method is useful in writing testcases, or when generating text.
format!() can be used to repeat single characters, but not repeating
strings like this.

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@bluss
Copy link
Member Author

bluss commented Sep 24, 2016

This method is called .rep(n: usize) -> String in crate odds, so it has been tested & used from there. It was also mentioned in this issue rust-lang/rfcs/issues/1253.

@frewsxcv
Copy link
Member

Compile errors

error: use of unstable library feature 'repeat_str' (see issue #0)
    --> src/libcollections/../libcollectionstest/str.rs:1291:19
     |
1291 |     assert_eq!("".repeat(3), "");
     |                   ^^^^^^
     |
     = help: add #![feature(repeat_str)] to the crate attributes to enable

error: use of unstable library feature 'repeat_str' (see issue #0)
    --> src/libcollections/../libcollectionstest/str.rs:1292:22
     |
1292 |     assert_eq!("abc".repeat(0), "");
     |                      ^^^^^^
     |
     = help: add #![feature(repeat_str)] to the crate attributes to enable

error: use of unstable library feature 'repeat_str' (see issue #0)
    --> src/libcollections/../libcollectionstest/str.rs:1293:20
     |
1293 |     assert_eq!("α".repeat(3), "ααα");
     |                    ^^^^^^
     |
     = help: add #![feature(repeat_str)] to the crate attributes to enable

error: aborting due to 3 previous errors

Build failed, waiting for other jobs to finish...
error: Could not compile `collections`.

@bluss
Copy link
Member Author

bluss commented Sep 24, 2016

Thanks. Fixed by amending. Yes I'm letting the bots do the compiling for me.

@pmarcelll
Copy link
Contributor

I have an interesting idea, would it make sense to add a collect_into method to iterators? It could take a mutable reference to a container, so the buffer can be preallocated. I know that it would require something different than FromIterator, so it should probably be on the Rust 2.0 wishlist, but I think it still make sense to add something like this.

@bluss
Copy link
Member Author

bluss commented Sep 24, 2016

@pmarcelll That sounds just like the existing String::extend(&mut self, iterable).

@pmarcelll
Copy link
Contributor

Oh, I knew about extend, but I didn't know it's completely generic. This is awesome!

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

Looks reasonable to me! I'd also be fine extending iterators/collections with more methods like this, but in isolation str::repeat seems like a fine method to have.

cc @rust-lang/libs

@alexcrichton
Copy link
Member

Let's see if this works...

@rfcbot disposition merge

Only thing I see missing is a tracking issue, but we can fill that in when it's agreed to land.

@alexcrichton
Copy link
Member

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Oct 3, 2016

FCP proposed with disposition to merge. Review requested from:

No concerns currently listed.
See this document for info about what commands tagged team members can give me.

@aturon
Copy link
Member

aturon commented Oct 4, 2016

I'm fine with this convenience (though I feel like we explicitly cut away some things like this on the road to 1.0).

@rfcbot
Copy link

rfcbot commented Oct 10, 2016

All relevant subteam members have reviewed. No concerns remain.

It is relatively simple to repeat a string n times:
`(0..n).map(|_| s).collect::<String>()`. It becomes slightly more
complicated to do it “right” (sizing the allocation up front), which
warrants a method that does it for us.

This method is useful in writing testcases, or when generating text.
`format!()` can be used to repeat single characters, but not repeating
strings like this.
@bluss
Copy link
Member Author

bluss commented Oct 10, 2016

Amended the commit to point to tracking issue #37079

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Oct 10, 2016

📌 Commit 2b7222d has been approved by alexcrichton

@bluss
Copy link
Member Author

bluss commented Oct 10, 2016

@bors rollup

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 11, 2016
Add method str::repeat(self, usize) -> String

It is relatively simple to repeat a string n times:
`(0..n).map(|_| s).collect::<String>()`. It becomes slightly more
complicated to do it “right” (sizing the allocation up front), which
warrants a method that does it for us.

This method is useful in writing testcases, or when generating text.
`format!()` can be used to repeat single characters, but not repeating
strings like this.
bors added a commit that referenced this pull request Oct 11, 2016
Rollup of 9 pull requests

- Successful merges: #36679, #36699, #36997, #37040, #37060, #37065, #37072, #37073, #37081
- Failed merges:
@bors bors merged commit 2b7222d into rust-lang:master Oct 12, 2016
@bluss bluss deleted the repeat-str branch October 12, 2016 08:27
@rfcbot
Copy link

rfcbot commented Oct 17, 2016

It has been one week since all blocks to the FCP were resolved.

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.

8 participants