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

Tidy up prelude #531

Merged
merged 3 commits into from May 9, 2019

Conversation

Projects
None yet
5 participants
@philandstuff
Copy link
Collaborator

commented May 6, 2019

Run Prelude through dhall format and remove usage of constructors from example code.

I think that this won't cause any hashes to change, so it shouldn't break anyone?

philandstuff added some commits May 6, 2019

run `dhall format` over Prelude
the `let` expressions indented in the old way was a little annoying
remove usage of `constructors` from example code
Some of the example code in Prelude used the now-removed `constructors`
keyword.  This removes it and runs the example through `dhall format`.
@philandstuff

This comment has been minimized.

Copy link
Collaborator Author

commented May 6, 2019

While I was doing this, I noticed that Prelude/JSON/Nesting could benefit from the new union-alternative-without-type syntax, but I thought this PR was big enough already.

@Gabriel439
Copy link
Contributor

left a comment

Thanks for doing this! At some point we should probably have CI enforce that the files are formatted

@Nadrieril

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

I had a question regarding the Prelude that could be relevant here: is there a particular reason the Prelude doesn't use itself ? For example, List/concatMap could use List/concat and List/map.

@singpolyma

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

@Nadrieril

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

@singpolyma I'm not sure what you mean: you can always import just concatMap if you want; the normal form might even stay the same.

@f-f

This comment has been minimized.

Copy link
Member

commented May 6, 2019

@Nadrieril my guess is that there is "code duplication" to minimize the amount of network calls involved: right now if you want to import concatMap you incur the cost of only one call, while if it depends on concat and map then you have to fetch three things instead, and I think this is also what @singpolyma meant

@Nadrieril

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

Ah right makes sense. Thanks !

@singpolyma

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

@Gabriel439

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

Yeah, the original reason for the Prelude not reusing itself was for fewer network calls, since it originated before Dhall supported semantic integrity checks. You'll see that some newer files (like the ./JSON directory) do have relative imports. Right now there's no good reason why they still don't import each other any longer

@philandstuff philandstuff merged commit dbcf50c into master May 9, 2019

1 check passed

hydra Hydra build #12213 of dhall-lang:531:dhall-lang
Details

@philandstuff philandstuff deleted the tidy-up-prelude branch May 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.