-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Add Option to stdlib #158
Add Option to stdlib #158
Conversation
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.
Thanks for this! I think it'd be nice to streamline a few of these implementations (see suggestions), but this looks good!
stdlib/option.gr
Outdated
| (Some(a), Some(b)) => Some((a, b)) | ||
| (Some(_), None) => None | ||
| (None, Some(_)) => None | ||
| (None, None) => None |
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.
| (Some(a), Some(b)) => Some((a, b)) | |
| (Some(_), None) => None | |
| (None, Some(_)) => None | |
| (None, None) => None | |
| (Some(a), Some(b)) => Some((a, b)) | |
| _ => None |
stdlib/option.gr
Outdated
| (Some(a), Some(b)) => Some(fn(a, b)) | ||
| (Some(_), None) => None | ||
| (None, Some(_)) => None | ||
| (None, None) => None |
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.
| (Some(a), Some(b)) => Some(fn(a, b)) | |
| (Some(_), None) => None | |
| (None, Some(_)) => None | |
| (None, None) => None | |
| (Some(a), Some(b)) => Some(fn(a, b)) | |
| _ => None |
stdlib/option.gr
Outdated
| Some(Some(x)) => Some(x) | ||
| Some(None) => None | ||
| None => None |
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.
| Some(Some(x)) => Some(x) | |
| Some(None) => None | |
| None => None | |
| Some(Some(x)) => Some(x) | |
| _ => None |
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.
Sweet! In the future we might move the Option
type into the compiler itself so other compiler types can use/return options (that'll be important when we have optional arguments) but for now this is perfect 👨🍳
stdlib/option.gr
Outdated
} | ||
} | ||
|
||
# TODO: Do you like `andThen` or should it be `flatMap` or both? |
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 would lean towards flatMap
, since the "then" part of andThen
feels like it could imply that something async is happening.
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.
+1. Plus, flatMap
is nice since Options are monads/functors
stdlib/option.gr
Outdated
} | ||
} | ||
|
||
# TODO: Would be cool to have a recursive version |
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 don't think the typechecker would let us write one without a custom type to match against :/
The type would have to look like this:
data RecOpt<a> = RecOptCons(Option<a>, RecOpt<a>) | RecOptNone
You could maybe get this to work with higher kinded types, but I'm not sure.
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.
Yeah, I'm not sure how calls to a recursive version would type-check in the first place...
@belph when I make your suggested changes, I get this warning:
How should I avoid that? |
I made the changes and rebased. We should figure out how to disable that warning for the stdlib maybe? |
Yeah, I'll make an issue. |
I wrote a thing to set the compiler's default warnings, and opened an issue so we can have them be configurable. (I made FragileMatch default to off, and left everything else as on for now.) |
This adds the
Option
data type to pervasives, and additionally adds anoption.gr
module to the standard library that shamelessly reimplements APIs I've liked from OCaml, BuckleScript's Belt library, and Rust.I also added
peek
because I think it is useful once there's currying and you can chain things with|>
or similar.One glaring omission from this implementation is converting to/from Result because that doesn't exist yet.