Add support for String CompareOptions on Unix#2196
Conversation
eerhardt
force-pushed
the
CompareOptions2
branch
from
cb68578 to
ab59f75
Compare
There was a problem hiding this comment.
Does the mutex need to be cleaned up when we're done with it with pthread_mutex_destroy?
There was a problem hiding this comment.
DOH. My mistake. Will add this to CloseSortHandle. Thanks for the catch.
There was a problem hiding this comment.
I've fixed this in the latest commit.
There was a problem hiding this comment.
Assert the return value for success?
eerhardt
force-pushed
the
CompareOptions2
branch
from
ab59f75 to
57cdd6b
Compare
|
LGTM. Thanks for doing this, Eric! |
There was a problem hiding this comment.
a nit, but IMO using 0x across all options would be more straightforward to signify bitwise fields
|
I've addressed all feedback. Thanks everyone. Unless more is received, I'll merge this change tomorrow around noon. |
|
@dotnet-bot test this please |
eerhardt
force-pushed
the
CompareOptions2
branch
from
a69e615 to
6bcf22d
Compare
…hat is passed in. This is in preparation of creating different UCollators for each option.
eerhardt
force-pushed
the
CompareOptions2
branch
2 times, most recently
from
5492baf to
6162dea
Compare
|
My previous code was broken on OSX (ICU 55.1). I fixed that issue, with lots of help from @markusicu. I did some more testing on Windows, and had to make a couple more changes to the rules. See the latest commit. @ellismg @steveharter - Can you take a look? |
|
@dotnet-bot test this please |
1. When IgnoreSymbols is true, ensure we still ignore half and fullwidth characters that are symbols. 2. Hiragana-Katakana characters differ at the tertiary strength, fixing the rule. 3. Fix collation on OSX which uses ICU 55.1. ICU 55 doesn't support having certain unicode characters using primary '<' rules. These characters are not necessary in the rules, since Windows always treats them the same. Removing 0x3099 and 0x309A from the half/full width rules.
eerhardt
force-pushed
the
CompareOptions2
branch
from
6162dea to
d966f66
Compare
|
@dotnet-bot test this please |
1 similar comment
|
@dotnet-bot test this please |
|
Test Ubuntu x64 Release Build and Test please |
|
@dotnet-bot test this please |
2 similar comments
|
@dotnet-bot test this please |
|
@dotnet-bot test this please |
|
OSX CI still seems broken. Merging. |
Add support for String CompareOptions on Unix
Add support for String CompareOptions on Unix Commit migrated from dotnet/coreclr@15706eb
5 participants
Adding support for IgnoreNonSpace, IgnoreSymbols, IgnoreKanaType and IgnoreWidth on Unix. The only option left undone is StringSort, which will be implemented in a future commit.
This is a partial fix for https://github.com/dotnet/corefx/issues/3858.
@ellismg @stephentoub @tarekgh