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

Implemented remaining string pattern API #23952

Merged
merged 6 commits into from Apr 7, 2015

Conversation

Kimundi
Copy link
Member

@Kimundi Kimundi commented Apr 1, 2015

This adds the missing methods and turns str::pattern in a user facing module, as per RFC.

This also contains some big internal refactorings:

  • string iterator pairs are implemented with a central macro to reduce redundancy
  • Moved all tests from coretest::str into collectionstest::str and left a note to prevent the two sets of tests drifting apart further.

See #22477

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

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

@bors
Copy link
Contributor

bors commented Apr 2, 2015

☔ The latest upstream changes (presumably #23955) made this pull request unmergeable. Please resolve the merge conflicts.

@Kimundi Kimundi force-pushed the more_string_pattern branch 4 times, most recently from ee8c14a to bfa1cab Compare April 2, 2015 17:38
///
/// The returned iterator will be double ended if the pattern allows a reverse search
/// and forward/reverse search yields the same elements. This is true for, eg, `char` but not
/// for `&str`.
Copy link
Member

Choose a reason for hiding this comment

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

Much of the surrounding style of documentation in this file is to wrap to 80 chars, could you also do that here?

@alexcrichton
Copy link
Member

Overall this looks good to me, but I'm finding it very difficult to follow with the large amount of macros being used, would you mind trying to reduce the number of macros in use? I think some extra verboseness is fine here as we're not really trying to optimize for disk space with the source code!

@Kimundi Kimundi force-pushed the more_string_pattern branch 2 times, most recently from 83bb01a to 76fad7e Compare April 4, 2015 13:12
- Added missing reverse versions of methods
- Added [r]matches()
- Generated the string pattern iterators with a macro
- Added where bounds to the methods returning reverse iterators
  for better error messages.
Fixed bug in existing StrSearcher impl
@alexcrichton
Copy link
Member

@bors: r+ fbba28e

Thanks!

@bors
Copy link
Contributor

bors commented Apr 7, 2015

⌛ Testing commit fbba28e with merge b2e65ee...

bors added a commit that referenced this pull request Apr 7, 2015
This adds the missing methods and turns `str::pattern` in a user facing module, as per RFC.

This also contains some big internal refactorings:
- string iterator pairs are implemented with a central macro to reduce redundancy 
- Moved all tests from `coretest::str` into `collectionstest::str` and left a note to prevent the two sets of tests drifting apart further.

See #22477
@bors bors merged commit fbba28e into rust-lang:master Apr 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants