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

Clean up toString implementation #9

Closed
travisbrown opened this issue Apr 13, 2020 · 1 comment · Fixed by #272
Closed

Clean up toString implementation #9

travisbrown opened this issue Apr 13, 2020 · 1 comment · Fixed by #272
Assignees
Labels
enhancement New feature or request maintainability

Comments

@travisbrown
Copy link
Owner

This is a follow-up to #7. I've now got toString for Expr working well enough that it can be used to round-trip the unnormalised prelude, which was my goal for 0.1.0, but the implementation is still a disaster. I'd originally been using it for debugging and didn't really care about producing valid Dhall code, and I just threw together the current version in the past couple of days. It parenthesises unnecessarily, probably still gets precedence wrong in some cases, is an unmaintainable mess, etc.

We'll also probably want some kind of Dhall code pretty-printing at some point, but I'll open a separate issue for that.

@travisbrown travisbrown added the enhancement New feature or request label Apr 13, 2020
@travisbrown travisbrown self-assigned this Apr 13, 2020
@travisbrown
Copy link
Owner Author

One specific thing to note is that we do have a property test in ToStringSuite that is supposed to validate that we can take arbitrary (not-necessarily-well-typed) expressions, print them with toString, and parse them to get the original value.

This test is ignored right now because it fails sometimes, but that could be an issue with the Arbitrary instance, and our Shrink isn't good enough to make the failure messages useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request maintainability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant