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 as_str method to more str-iterators #34204

Closed
wants to merge 1 commit into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 10, 2016

In the spirit of Chars::as_str

@rust-highfive
Copy link
Collaborator

r? @brson

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

fn as_str(&self) -> &'a str {
match self.searcher {
StrSearcherImpl::Empty(ref searcher) => &self.haystack[searcher.position..searcher.end],
StrSearcherImpl::TwoWay(ref searcher) => &self.haystack[searcher.position..searcher.end],
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this line is too long (> 100 chars) 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@brson brson added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 11, 2016
@brson
Copy link
Contributor

brson commented Jun 11, 2016

Floodgates are opened I guess. grumble.

@alexcrichton
Copy link
Member

I think that this is a breaking change due to the addition of a non-default method on Searcher, right?

@alexcrichton
Copy link
Member

Breaking change for nightly users, that is.

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 13, 2016

Floodgates are opened I guess. grumble.

:/ I'm not depending on this, I just noticed the discrepancy. Feel free to close this PR and request an RFC if you think this should be properly designed.

An alternative would be to deprecate the as_str methods and create an RFC on how to properly get the remaining str or [T] or other ranged type for iterators (e.g. some might want to not drop the allocation in a Vec::into_iter, but get it back).

Breaking change for nightly users, that is.

indeed

@bluss
Copy link
Member

bluss commented Jun 27, 2016

There are forward and reverse iterators (string searchers) that are not double ended. Those correspond to two different string slices (one from the front cursor until the end, the other from the back cursor until the start)

@alexcrichton
Copy link
Member

Discussed during libs triage yesterday, in light of @bluss's realization and lack fo strong motivation for this we're going to close at this time, but thanks though for the PR @oli-obk!

@oli-obk oli-obk deleted the iter_as_str branch June 28, 2016 17:15
&self.haystack[searcher.position..searcher.end]
},
StrSearcherImpl::TwoWay(ref searcher) => {
&self.haystack[searcher.position..searcher.end]
Copy link
Member

Choose a reason for hiding this comment

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

For example, position and end have no relation and position > end is a legal state.

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.

6 participants