Skip to content
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!: Char #474

Merged
merged 1 commit into from
Dec 16, 2020
Merged

feat!: Char #474

merged 1 commit into from
Dec 16, 2020

Conversation

ospencer
Copy link
Member

Char

This PR adds the Char type to Grain. Like other hip modern languages, a Grain Char is a single Unicode scalar value instead of a single byte. 😎 (For single bytes, we'll probably introduce a Byte type.)

The Char Stdlib

This PR also includes some char utilities in a new Char stdlib.

// The minimum value of Unicode characters
min: Number

// The maximum value of Unicode characters
max: Number

// Returns true if the given number is a valid Unicode scalar value.
// @param n: Number - the value to check
// @returns Bool
isValid

// Returns the Unicode code point for the character
// @param char: Char - the input character
// @returns Number
code

// Returns the Char for the given code point. Fails if the code point is invalid.
// @param codePoint: Number - the Unicode code point
// @returns Char
fromCode

// Returns the next valid Unicode character by code point. Fails if the input character is U+10FFFF.
// @param char: Char - the input character
// @returns Char
succ

// Returns the previous valid Unicode character by code point. Fails if the input character is U+0000.
// @param char: Char - the input character
// @returns Char
pred

Breaking Changes

This PR is a breaking change because String.explode was updated to return an array of characters instead of strings.

Character Literal Syntax

There is currently no syntax to create character literals, as we have yet to decide on a syntax.

@ospencer ospencer requested a review from a team December 14, 2020 20:00
@ospencer ospencer self-assigned this Dec 14, 2020
Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Just a couple questions

Comment on lines +31 to +40
let succ = (c) => {
let codePoint = code(c)
if (codePoint == max) {
fail 'no valid Unicode code point past U+10FFF'
} else if (codePoint == 0xD7FF) {
fromCode(0xE000)
} else {
fromCode(codePoint + 1)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts on succ and pred returning Options?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since these only fail in one pretty specific (and uncommon) case, it didn't really seem necessary to me. Feels like the kind of thing you would always just Option.get without doing any checking.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, let's make sure to document it and give the reasoning somewhere. I think that's especially important for contributor docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, for sure.

export function fromCode(code: u32): u32 {
code = code >>> 1
if (code < 0x80) {
let char = allocateChar()
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for having this twice instead of once at the top?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really. This way feels a tad more clear to me since all of the code for the "fast" case is contained within the if. We'd get a few bytes back if we moved it up though.

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine. Was just curious.

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Reasoning seem good to me! 👍

@ospencer ospencer merged commit d665ce7 into master Dec 16, 2020
@ospencer ospencer deleted the char branch December 16, 2020 17:32
ospencer added a commit that referenced this pull request Jan 11, 2021
ospencer added a commit that referenced this pull request Jan 16, 2021
ospencer added a commit that referenced this pull request Jan 16, 2021
ospencer added a commit that referenced this pull request Jan 16, 2021
ospencer added a commit that referenced this pull request Jan 16, 2021
@github-actions github-actions bot mentioned this pull request Apr 20, 2021
@github-actions github-actions bot mentioned this pull request May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants