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

Improve error messages for list elements with the wrong type #236

Merged
merged 1 commit into from
Jan 28, 2018

Conversation

Gabriella439
Copy link
Collaborator

Fixes #230

The type error for the list now returns the expression for the mismatched
element instead of the entire list. This improves the source span
displayed to the user to focus on the invalid element.

Since the source span now matches the invalid element, there is no need
to redisplay the same element in the detailed error message.

Fixes #230

The type error for the list now returns the expression for the mismatched
element instead of the entire list.  This improves the source span
displayed to the user to focus on the invalid element.

Since the source span now matches the invalid element, there is no need
to redisplay the same element in the detailed error message.
Copy link
Member

@ocharles ocharles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but have you considered some golden tests the test each of these error messages? It also works as a nice design tool, as people can easily review all error messages and easily provide some commentary on how the messages could be improved.

@Gabriella439
Copy link
Collaborator Author

The only reason I haven't done this yet is because the error messages are not part of the API contract and they are pretty brittle to maintain

I think if I did this it would be to add functions for automating common error message patterns (i.e. wrapping code in ASCII boxes, wrapping to 80 columns) and testing those functions.

@Gabriella439 Gabriella439 merged commit 11219e7 into master Jan 28, 2018
@Gabriella439 Gabriella439 deleted the gabriel/230 branch January 28, 2018 17:45
@ocharles
Copy link
Member

Golden tests don't really imply that, they are just a mega easy way to test things which have a known output. That output can still change over time, of course, it's just trivial to say "yea yea, my changes are what I wanted". I've come to really enjoy them. In a sense, golden testing the error messages against Dhall expressions also stresses the type checker, too.

@Gabriella439
Copy link
Collaborator Author

Alright, but there's still one other thing that has to be implemented, which is an easy way to update the tests to accept the new golden output. For example, if you significantly refactor error messages and 50 tests start failing, it would be time-consuming to update all 50 of them manually.

@ocharles
Copy link
Member

With golden testing, you have a runner that just accepts the new result. With tasty-golden, the typical flow is to review the differences, and then just re-run with --accept, which rewrites the golden output files.

@Gabriella439
Copy link
Collaborator Author

Ah, I didn't know that tasty-golden supported something like that. I'll have to try it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants