-
Notifications
You must be signed in to change notification settings - Fork 211
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
dhall-json: Nesting: Support empty alternatives as contents #1204
Conversation
alternativeName | ||
) | ||
contents | ||
, contents | ||
) | ||
, ( "field" |
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.
The additional indentation here confused me for so long, I ended up opening a GHC issue! ;)
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, the indentation there bugged me, too 🙂
) | ||
contents | ||
] | isInlineNesting nesting | ||
, Just (alternativeName, Just (Core.RecordLit kvs)) <- getContents contents -> do |
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 wonder whether we should report an error in this case:
, Just (alternativeName, Just (Core.RecordLit kvs)) <- getContents contents -> do | |
, Just (alternativeName, Just notARecordLit) <- getContents contents -> do |
Non-record alternatives are supported with Nested
, so users might also try them with Inline
.
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.
@sjakobi: Yeah, we probably should report an error here. It's up to you if you want to include that in this pull request or not
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'll add the error in a follow-up PR.
alternativeName | ||
) | ||
contents | ||
, contents | ||
) | ||
, ( "field" |
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, the indentation there bugged me, too 🙂
) | ||
contents | ||
] | isInlineNesting nesting | ||
, Just (alternativeName, Just (Core.RecordLit kvs)) <- getContents contents -> do |
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.
@sjakobi: Yeah, we probably should report an error here. It's up to you if you want to include that in this pull request or not
Thank you! |
Closes #1201.