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

Error Message Potpourri #3429

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

mattpolzin
Copy link
Collaborator

@mattpolzin mattpolzin commented Nov 30, 2024

Description

I tried to keep the following isolated in their own commits so we can just ditch anything folks don't like.


Whereas the CLI for the REPL currently only pays attention to one argument as an input file but quietly ignores the rest, I've made it an error to specify multiple input files:

Uncaught error: Error: Expected at most one input file but was given: file1.idr, file2.idr

This is further improved (described in the last section below) by using die instead of an uncaught throw so the error doesn't look so unexpected.


Whereas the REPL previously printed an error about a file loaded with :l in monochrome without fanfare, I've indented the error message, given it room to stand out, and written the context of the error in the standard IdrisAnn color for error messages.

Main> :l "file1.idr"

  Error loading file "file1.idr": File Not Found

Main> [...]

Whereas on launch the REPL previously printed a subtly different error when a requested file could not be loaded than it did when using the :l command inside the REPL, I've aligned the two errors (and indeed refactored them to use the same code). I've also added a suggestion specifically for this startup use-case because it is common to see people intend to run e.g. idris2 --repl ... but instead type idris2 repl ... which causes the REPL to attempt to load a file named "repl" -- the suggestion is that the user may have meant to write idris2 --repl ... so this should ease the pain of a common mistake.

 $ ./build/exec/idris2 repl
     ____    __     _         ___
    /  _/___/ /____(_)____   |__ \
    / // __  / ___/ / ___/   __/ /     Version 0.7.0-c1fc0b346
  _/ // /_/ / /  / (__  )   / __/      https://www.idris-lang.org
 /___/\__,_/_/  /_/____/   /____/      Type :? for help

Welcome to Idris 2.  Enjoy yourself!

  Error loading file "repl": File Not Found
  Did you mean to type "--repl"?

Main> [...]

As mentioned above, this PR finally switches from uncaught throw for rendering errors from the CLI over to using die so we get a less buggy looking printout. This commit also adds in a suggestion when relevant similar to the one seen within the REPL session for the above case.

 $ ./build/exec/idris2 build idris2.ipkg
Error: Expected at most one input file but was given: build, idris2.ipkg
Did you mean to type "--build"?

Should this change go in the CHANGELOG?

  • If this is a fix, user-facing change, a compiler change, or a new paper
    implementation, I have updated CHANGELOG_NEXT.md (and potentially also
    CONTRIBUTORS.md).

@mattpolzin mattpolzin force-pushed the err-msgs branch 3 times, most recently from 93d9004 to b4fd807 Compare December 1, 2024 23:48
…they were trying to run a command but failed to use the double-dash syntax
- make error `die` instead of being an unhandled throw.
- give error a suggestion if one of the file names looks like it was
  intended to be a CLI option.
- update tests now that some existing errors are not reported as unhandled anymore (and they are printed to stderr).
- remove now-redundant "Error: " prefix from `UserError` rendering.
- make sure no-color options have been set from environment before outputting first formatted error from `stMain`.
@mattpolzin mattpolzin marked this pull request as ready for review December 2, 2024 01:42
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.

1 participant