-
Notifications
You must be signed in to change notification settings - Fork 51
Add UniCase::contains()
#52
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
base: master
Are you sure you want to change the base?
Conversation
|
||
loop { | ||
let p = match pat.next() { | ||
None => break 'out true, |
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.
All these breaks could just be returns right? I think that would be simpler
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.
In fact, depending on if there's a minimal supported rust version, you could even write
let Some(p) = pat.next() else { return true };
(The build.rs seems to imply there is, so this probably is too fancy).
What is the status of this PR? Is this simply waiting for the feedback in question to be applied to take it over the finish line? If so I would be happy to do the honors |
#[inline] | ||
pub fn contains<S2: AsRef<str>>(&self, pat: &UniCase<S2>) -> bool { | ||
match (&self.0, &pat.0) { | ||
(&Encoding::Ascii(ref x), &Encoding::Ascii(ref p)) => x.as_ref().contains(p.as_ref()), |
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.
Won't this in practice be a case sensitive contains?
Adds
UniCase::contains()
to test if one unicode string contains another.This somehow conflict with / workaround #47