Made char.IsAscii public #41396
Made char.IsAscii public #41396
Conversation
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
| Assert.True(char.IsAscii(char.MinValue)); | ||
| Assert.True(char.IsAscii('\x007f')); | ||
| Assert.False(char.IsAscii('\x0080')); | ||
| Assert.False(char.IsAscii(char.MaxValue)); |
There was a problem hiding this comment.
Is this enough for testing, or should be use the "whole range" ala
?
There was a problem hiding this comment.
The whole range. There is probably a similar test for Rune.IsAscii
There was a problem hiding this comment.
Just scanned again through the repo, looking for IsAscii-tests. Haven't found one.
I've seen that all IsAscii implementations, like
delegate toIt's a trivial method (just one compare), but maybe it's better to delegate char.IsAscii also to this method and add tests over there (as I didn't find some for IsAsciiCodePoint -- maybe because it's defined in an internal type)?
There was a problem hiding this comment.
Search the file https://github.com/dotnet/runtime/blob/master/src/libraries/System.Runtime/tests/System/Text/RuneTests.cs for the substring "IsAscii". You'll see how we perform tests against the Rune APIs.
Though to be honest those tests are basically just testing specific values like '\u0000', '\u007f', etc., as you had here. :)
I have no problem with you leaving the test as you have it here.
|
This needs to be also exposed in the /ref/ file where Char surface is defined. |
|
Ahhh, ref-assembly -- thanks for the hint / info. |
| Assert.True(char.IsAscii(char.MinValue)); | ||
| Assert.True(char.IsAscii('\x007f')); | ||
| Assert.False(char.IsAscii('\x0080')); | ||
| Assert.False(char.IsAscii(char.MaxValue)); |
There was a problem hiding this comment.
Search the file https://github.com/dotnet/runtime/blob/master/src/libraries/System.Runtime/tests/System/Text/RuneTests.cs for the substring "IsAscii". You'll see how we perform tests against the Rune APIs.
Though to be honest those tests are basically just testing specific values like '\u0000', '\u007f', etc., as you had here. :)
I have no problem with you leaving the test as you have it here.
|
@GrabYourPitchforks I pushed the commit with the ref-assembly update.
I left the test as they are. |
ghost
locked as resolved and limited conversation to collaborators
Fixes #40936
Also normalized parameters of other method to
c(instead ofch, only two others)./cc: @GrabYourPitchforks