-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
feat(stdlib)!: Add an Ascii
submodule to Char
and move isAscii
, toUppercase
, toLowercase
#2178
base: main
Are you sure you want to change the base?
Conversation
I'm in favor of an |
isAscii
,isAsciiControl
,isAsciiWhitespace
to Char
Ascii
submodule to Char
and move isAscii
, toUppercase
, toLowercase
Done |
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.
This is awesome. Really looking forward to this.
* | ||
* @example assert Char.isAsciiDigit('1') | ||
* @example assert !Char.isAsciiDigit('a') | ||
* @example Char.Ascii.isAscii('1') |
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.
I think it's fine to do the examples as just Ascii.
without the leading Char.
.
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.
I think we should keep the Char.Ascii.isAscii
, I feel like it's not very long and it makes the examples runnable, in other modules such as array I was using the pattern:
use Array.{ module Immutable }
assert Immutable.isEmpty(Immutable.empty) == true
but I felt that the module name ascii was small enough here to just directly inline it. I think there is real value in having our examples be as runnable as possible while still being small.
* | ||
* @since v0.7.0 | ||
*/ | ||
provide let min = 0x00 |
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.
Shouldn't these be chars? If someone really needs the code then they can do a Char.code
on it.
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.
We currently use the code on the top level module Char.min
. I think if we really want to change that we should do it in a seperate pr after this. Though there is the argument here that min
and max
are odd values for chars whereas they make more sense in relation to charCodes. maybe we have both minCode
, maxCode
and min
, max
?
Co-authored-by: Oscar Spencer <oscar.spen@gmail.com>
Co-authored-by: Oscar Spencer <oscar.spen@gmail.com>
This pr adds some additional
Ascii
helper functions to theChar
module, it also moves the existingAscii
utilities into a newAscii
submodule withinChar
.Breaking: This is a breaking change as
isAscii
,toAsciiLowercase
,toAsciiUppercase
,isAsciiDigit
andisAsciiAlpha
were all moved and renamed within the submodule.