Skip to content

Unicode-compatible (mostly) hasPrefix and hasSuffix #239

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

Merged
merged 3 commits into from
Jan 26, 2016
Merged

Unicode-compatible (mostly) hasPrefix and hasSuffix #239

merged 3 commits into from
Jan 26, 2016

Conversation

glessard
Copy link
Contributor

The version of hasPrefix and hasSuffix originally found in Foundation for Linux does not do well with Unicode strings. The implementations is this PR reproduce those supplied in the standard library in the presence of the Objective-C runtime, namely swift_stdlib_NSStringHasPrefixNFD and swift_stdlib_NSStringHasSuffixNFD. The test results are largely the same, that is the issues appear to be the same (except the issues related to embedded nulls.)

Closing #238, as it is superseded by this.

return true
let cfstring = self._cfObject
let range = CFRangeMake(0, CFStringGetLength(cfstring))
let opts = CFStringCompareFlags(kCFCompareAnchored + kCFCompareNonliteral)
Copy link
Contributor

Choose a reason for hiding this comment

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

+? shouldn't that be an |?
This might be cleaner just to funnel to the NS version of
rangeOfString(str, options: .AnchoredSearch, range: NSMakeRange(0, length)).location != NSNotFound

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, a bitwise-or is more logical.

@glessard
Copy link
Contributor Author

@phausler
An NS version wouldn't exhibit the same behaviour as the CF version. Either the behaviour of the stdlib version should be duplicated (as this PR does), or the behaviour should be as correct as possible.
swiftlang/swift#1031 has a ICU-based solution that behaves better, but also behaves better than the _runtime(_ObjC) version.

@phausler
Copy link
Contributor

The NS version does nearly exactly what you are doing:
public func hasPrefix(str: String) -> Bool {
return rangeOfString(str, options: .AnchoredSearch, range: NSMakeRange(0, length)).location != NSNotFound
}
which rangeOfString calls
CFStringFindWithOptionsAndLocale(_cfObject, searchString._cfObject, CFRange(searchRange), mask._cfValue(true), nil, ranges)

@glessard
Copy link
Contributor Author

I thought I remembered the test results being different when I tried it, but maybe I just decided in favor of the lower-level call. In any event, this version is very close to the one found at https://github.com/apple/swift/blob/master/stdlib/public/stubs/SwiftNativeNSXXXBase.mm.gyb#L131

phausler added a commit that referenced this pull request Jan 26, 2016
Unicode-compatible (mostly) hasPrefix and hasSuffix
@phausler phausler merged commit 8f658e1 into swiftlang:master Jan 26, 2016
@glessard glessard deleted the unicode-prefix-suffix branch February 4, 2016 23:31
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.

2 participants