-
Notifications
You must be signed in to change notification settings - Fork 219
dhall-nix: Fix field-based union access #907
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
Conversation
dhall-nix/src/Dhall/Nix.hs
Outdated
| k' <- Dhall.Map.keys kts | ||
| return (k', Nothing) | ||
| let e2 = Fix (NSym k) | ||
| return (Fix (NAbs (ParamSet e0 False Nothing) e2)) |
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.
If I understand correctly, this compiles < Foo >.Foo to { Foo }: Foo, but I believe the result should instead be: x: { Foo }: Foo x
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.
<Foo>.Foo compiling to {Foo}: Foo seems correct to me, and is the current behavior. What is that x about?
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.
Oh, you're right. Never mind, my thoughts were confused
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.
Now I remember what led to my confusion. The issue here is that you need to check if the alternative is empty when compiling the union to Nix.
For example:
dhallToNix "< Foo >.Foo" = { Foo }: Foo
dhallToNix "< Foo : {} >.Foo {=}" = { Foo }: Foo {}
dhallToNix "< Foo : {} >.Foo" = x: { Foo }: Foo xCarefully note the difference between the 1st and 3rd examples. If the type of the alternative is present then you need to turn it into an anonymous function.
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.
Ah, so it's a case of < .. >.x being partially applied when the alternative does take data
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.
Ok, I believe my latest commit resolves this.
|
Here's the latest behaviour: |
|
@ocharles: I think you need to import |
|
Ah no, I just need to push another commit |
Fixes #906.
I copied the approach used in
dhall-json.