Refactor config file & custom rewrites #225
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and make more config fields optional
+ be consistent with commas in cabal config
|
Done with this first refactor. Feedback welcome. We don't have tests for config files yet but I won't make it part of this PR. All commits more or less self-standing and passing tests. |
jespercockx
approved these changes
Nov 13, 2023
| RecordWildCards | ||
| FlexibleContexts | ||
| MultiWayIf | ||
| TupleSections |
There was a problem hiding this comment.
Oh wow, you can skip the commas? TIL.
There was a problem hiding this comment.
Yes! The syntax is surprisingly flexible (unfortunately, probably), though the documentation seems incorrect. I tried trailing commas for build-depends and it wasn't allowed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Been looking at more of the source code to try and understand what's going on before I start implementing the enhancements mentioned earlier.
I had missed the fact that we now have support of custom user rewrites. The code seemed very hard to maintain so I went ahead and simplified the parsing of config files (now depending on
yamlandaeson, the latter already being a dependency of Agda).I changed the flag
--rewrite-rulesto--configas it appears the file already contains more than rewrite rules (e.g: prelude import options). And it's likely we will use the config for more.The config isn't parsed using
unsafeIOanymore when parsing CLI options, but rather fetched safely using thepreCompilehook of the backend.Still need to update the documentation.