-
Notifications
You must be signed in to change notification settings - Fork 217
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
updated documentation strings to include empty tag in a union #2625
base: main
Are you sure you want to change the base?
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.
I think this is what you want
dhall/src/Dhall/Marshal/Encode.hs
Outdated
adapt (Queued n) = Just (Left n) | ||
adapt (Result t) = Just (Right (Left t)) | ||
adapt (Errored e) = Just (Right (Right e)) | ||
adapt Unreachable = Nothing |
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.
adapt (Queued n) = Just (Left n) | |
adapt (Result t) = Just (Right (Left t)) | |
adapt (Errored e) = Just (Right (Right e)) | |
adapt Unreachable = Nothing | |
adapt (Queued n) = Left n | |
adapt (Result t) = Right (Left t) | |
adapt (Errored e) = Right (Right (Left e)) | |
adapt Unreachable = Right (Right (Right ())) |
dhall/src/Dhall/Marshal/Encode.hs
Outdated
adapt (Queued n) = Just (Left n) | ||
adapt (Result t) = Just (Right (Left t)) | ||
adapt (Errored e) = Just (Right (Right e)) | ||
adapt Unreachable = Nothing |
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.
adapt (Queued n) = Just (Left n) | |
adapt (Result t) = Just (Right (Left t)) | |
adapt (Errored e) = Just (Right (Right e)) | |
adapt Unreachable = Nothing | |
adapt (Queued n) = Left n | |
adapt (Result t) = Right (Left t) | |
adapt (Errored e) = Right (Right (Left e)) | |
adapt Unreachable = Right (Right (Right ())) |
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 I am getting a bit lost. The documentation on Hackage --- I think it is different from the docstring in the source file which I have been editing. Is this a newer version? What does the adapt
function do?
That's why I wanted to compile it, to see the up-to-date documentation. But I could not compile 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.
Yeah, it's been a while since the last release to Hackage, which is the reason for the difference. However, I'm working on cutting a new release soon
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.
Stackage would also be great. It would enable Hoogle...
dhall/src/Dhall/Marshal/Encode.hs
Outdated
@@ -943,6 +943,7 @@ encodeFieldWith name encodeType = RecordEncoder $ Dhall.Map.singleton name encod | |||
data Status = Queued Natural | |||
| Result Text | |||
| Errored Text | |||
| Unreachable () |
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.
| Unreachable () | |
| Unreachable |
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 how I had it:
data DateFinished = Current () | Done DATE.Day | Dropped () deriving (Generic, Show, Eq)
and then:
dateFinishedDecoder :: Decoder DateFinished
dateFinishedDecoder =
union
( (Current <$> constructor "Current" unit)
<> (Done <$> constructor "Done" day)
<> (Dropped <$> constructor "Dropped" unit)
)
But if I do as you suggested, i.e. data DateFinished = Current | Done DATE.Day | Dropped deriving (Generic, Show, Eq)
--- then it does not compile
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 you change that to:
dateFinishedDecoder :: Decoder DateFinished
dateFinishedDecoder =
union
( (Current <$ constructor "Current" unit) -- Carefully note the `<$`
<> (Done <$> constructor "Done" day)
<> (Dropped <$> constructor "Dropped" unit)
)
… then it should compile
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, I got it ! Cute (although smells of witchcraft...). Then, maybe, instead of my suggested patch, would you rather write something pedagogical, in that docstring?
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 just requested the pull for the correcting patch, as you suggested.
The existing documentation on
union
decoder misses the case when one of constructors is an empty tag. This patch adds an example of that.