Skip to content
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

Move syntax things from Dhall.Core to a new module Dhall.Syntax #1440

Merged
merged 1 commit into from
Oct 19, 2019

Conversation

sjakobi
Copy link
Collaborator

@sjakobi sjakobi commented Oct 19, 2019

This partially addresses #1127.

@sjakobi sjakobi force-pushed the sjakobi/dhall-syntax branch 2 times, most recently from 25006d2 to 3f9050e Compare October 19, 2019 05:35
@Gabriella439
Copy link
Collaborator

@sjakobi: I think my main question is: what is the criterion for what belongs in Dhall.Syntax vs. Dhall.Core? For example, some of the utilities in Dhall.Syntax seem like they could reside in Dhall.Core (like denote or subExpressions)

@sjakobi
Copy link
Collaborator Author

sjakobi commented Oct 19, 2019

For denote, the motivation was to have only a single import cycle with Dhall.Pretty.Internal. Dhall.Syntax already has a hs-boot file. If denote would have stayed in Dhall.Core, we'd need to have an additional hs-boot file for Dhall.Core.

For the optics, the reasoning was that these always change together with the types, so I chose to locate them as low as possible in the module hierarchy.

I can document this in the module header.

Note that I plan to move the normalization code to another new module Dhall.Normalize. Dhall.Core would then only contain a few "miscellaneous" things, and re-export Dhall.Syntax and Dhall.Normalize.

@mergify mergify bot merged commit bec6ce6 into master Oct 19, 2019
@mergify mergify bot deleted the sjakobi/dhall-syntax branch October 19, 2019 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants