API Proposal: Make char.IsAscii public

[eiriktsarpalis]

System.Char currently defines a private IsAscii method. It think it might make sense to make this public?

public readonly struct Char
{
+  public static bool IsAscii(char ch);
}

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[eiriktsarpalis]

Not sure what the right area label for corelib stuff is.

[danmoseley]

Note there is already Rune.IsAscii(char) but it seems to me this is probably a case where duplication is reasonable, as the duplication is at a trivial level, and Rune is much less widely used than char; one might operate entirely on char and only need this one method on Rune.

[danmoseley]

@GrabYourPitchforks

[terrajobst]

Unless @GrabYourPitchforks has any objections, this feels like a sensible addition.

[GrabYourPitchforks]

We're also adding Ascii.IsAscii(char) (see #28230).

But I agree, targeted duplication like what's suggested here could help discoverability.

[terrajobst]

Video

• Looks reasonable

namespace System
{
    public readonly struct Char
    {
        public static bool IsAscii(char ch);
    }
}

[GrabYourPitchforks]

Correction: the parameter should be named c, not ch. That matches other static APIs on the char class.

[danmoseley]

Anyone in the community interested in picking this up?

[gfoidl]

You can assign it to me -- will send a PR tomorrow.

Is https://github.com/dotnet/runtime/blob/master/src/libraries/System.Runtime/tests/System/CharTests.cs the right place to add some tests for this?

That matches other static APIs on the char class.

There is a bit of mix on c / ch. Shall this be unified to c (within the coming PR)?

[GrabYourPitchforks]

@gfoidl it's yours! :)

There is a bit of mix on c / ch. Shall this be unified to c (within the coming PR)?

Looks like we use c for our public APIs and ch (sometimes) for our internal APIs? I can't find any counterexamples offhand but admittedly I didn't look that hard. I wouldn't bother normalizing any other internal methods during this PR.

[svick]

@GrabYourPitchforks

For the record, in .Net Core 3.1 corelib, I count 29 methods with char c parameter and 9 methods with char ch:

• System.Text.Rune: System.Text.Rune op_Explicit(Char)
• System.Text.Rune: Boolean TryCreate(Char, System.Text.Rune ByRef)
• System.Text.Rune: Void .ctor(Char)
• System.IO.BinaryWriter: Void Write(Char)
• System.Globalization.CharUnicodeInfo: Double GetNumericValue(Char)
• System.Globalization.CharUnicodeInfo: Int32 GetDecimalDigitValue(Char)
• System.Globalization.CharUnicodeInfo: Int32 GetDigitValue(Char)
• System.Globalization.CharUnicodeInfo: System.Globalization.UnicodeCategory GetUnicodeCategory(Char)
• System.Globalization.CompareInfo: Boolean IsSortable(Char)

[GrabYourPitchforks]

@svick right, I'm only considering existing static public methods on the char type. That way the parameter names are at least locally consistent.

[gfoidl]

Are there any steps needed for the documentation?

[danmoseley]

@gfoidl it's complicated -- normally the simple answer is "add XML docs in the src for this new public method" and that may be the correct answer. It's complicated because unlike almost everyone else, the core libraries don't historically generate their docs from the sources. We do want to use XML docs going forward, but only for seeding the docs.microsoft.com text initially. We have a tool that does this seeding so I think the answer is we'll need another PR to add <summary> for IsAscii. We missed this in PR review.

Now I've typed that, I see it's documented by @carlossanlop here https://github.com/dotnet/runtime/blob/c87e75e8ef260ccc6e979f9df1cc012536ee036f/docs/coding-guidelines/adding-api-guidelines.md#documentation

[gfoidl]

@danmosemsft it was easy to miss, as no other api in

runtime/src/libraries/System.Private.CoreLib/src/System/Char.cs

Line 25 in 756addd

public readonly struct Char : IComparable, IComparable<char>, IEquatable<char>, IConvertible

has xml doc comments.

So I'll send a PR with the doc comments (for this new public API)?

[danmoseley]

@gfoidl certainly that is true, and it is because until recent years, there was no path from our source code to docs at all, and I guess we have not added much to Char recently. Yes, all that is needed is one for the new member. thanks