Skip to content

provide more helpful error message when running an improperly-typed term - #1814

Merged
aryairani merged 2 commits into
unisonweb:trunkfrom
samgqroberts:1747.run-type-error-message
Feb 25, 2021
Merged

provide more helpful error message when running an improperly-typed term#1814
aryairani merged 2 commits into
unisonweb:trunkfrom
samgqroberts:1747.run-type-error-message

Conversation

@samgqroberts

Copy link
Copy Markdown
Contributor

Overview

Endeavoring to fix #1747 - see the output of the new transcript for what the new error message is and when it is shown.

Implementation notes

Instead of HandleInput.addRunMain just returning a Maybe (I could do it, or I couldn't), it now returns a type that disambiguates whether it couldn't due to not being able to find the term, or whether the term has a bad type. That new case is subsequently handled in the error reporting module OutputMessages.hs to produce the new error message.

Interesting/controversial decisions

Test coverage

I've included a new transcript run.md that attempts to run properly-typed, improperly-typed, and nonexistent terms to show the differences in behavior / error messages.

Loose ends

  • This PR is obfuscated by another bug - you'll see me call this out in the new transcript. I believe I'm running into Regression in run - fails to find the function to run #1800, and I've confirmed that without my changes that runnable function still fails to be found. I wanted to put this PR up optimistically to gather feedback while that issue is resolved.
  • I didn't update the error reporting in Execute.hs to provide this distinction (partly due to my lack of familiarity of what experience that file is part of..)

In general, this being my first contribution, I'm not confident at all that the changes I've provided are idiomatic / sufficient. I'd be more than happy to add tests, move code, rename types. Thank you!

@pchiusano

Copy link
Copy Markdown
Member

This is great, a nice little usability improvement! Can you add yourself to CONTRIBUTORS.markdown? I'll wait for CI to pass before merging.

@aryairani aryairani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@aryairani
aryairani merged commit 666919b into unisonweb:trunk Feb 25, 2021
@pchiusano pchiusano mentioned this pull request May 11, 2021
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.

run reports that a given term does not exist if that term does not have the proper type

3 participants