-
-
Notifications
You must be signed in to change notification settings - Fork 173
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 showConstructor
to convert union value constructor to Text
#1257
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.
Looks great! I think all this needs is tests
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.
Looks good to me, just this one detail.
* Fix grammar spacing * Make typing rule less strict to work with abstract arguments Co-authored-by: Gabriella Gonzalez <Gabriel439@gmail.com> Co-authored-by: Verity Scheel <monoidmusician@gmail.com>
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.
Looks like a great feature! :)
I've been trying to think of some "interesting" type-inference failure tests. So far I've only come up with:
showConstructor < A : Bool >.A
showConstructor None
Please also add type-inference and normalization success tests for something like showConstructor (< A : Bool>.A False)
.
I also wonder whether showConstructor
ought to work for Bool
s. I don't see why not. Our main reason for not allowing merge
for Bool
s doesn't seem to apply in this case.
Thanks for the test suggestions! I've added those.
|
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.
showConstructor for Bools sounds reasonable, though I wonder what the use case would be where you wouldn't just use Bool/show.
It might be just slightly more convenient than importing Bool/show
from the Prelude. Not a very important addition though.
tests/type-inference/success/unit/ShowConstructorNonEmptyB.dhall
Outdated
Show resolved
Hide resolved
tests/normalization/success/unit/ShowConstructorNonEmptyB.dhall
Outdated
Show resolved
Hide resolved
Co-authored-by: Simon Jakobi <simon.jakobi@gmail.com>
… as standardized in dhall-lang/dhall-lang#1257 Co-authored-by: David Richey <darichey1@gmail.com>
Fixes #1204
Adds a
showConstructor
keyword that can be used to convert a union value toText
by its constructor name. For example:It also works on
Optional
values:I have a draft implementation for dhall-haskell: darichey/dhall-haskell@64ef9df