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 the "did you mean" suggestions for booleans #198

Merged
merged 1 commit into from
Oct 8, 2023

Conversation

eminence
Copy link
Contributor

@eminence eminence commented Oct 8, 2023

If you misspell true or false, you get weird "did you mean" suggestions:

>>> 4 > 5 == False
error: while type checking
  ┌─ <input:7>:1:10
  │
1 │ 4 > 5 == False
  │          ^^^^^ unknown identifier
  │
  = Did you mean 'floz'?

This is because true and false are keywords, not identifiers, so the "did you mean" code doesn't consider them.

This small patch appends both values to the list sent to did_you_mean().

An alternative way to fix this (arguably a better way) would be to modify the impl ErrorDiagnostic for TypeCheckError to check for other suggestions on a TypeCheckError::UnknownIdentifier error

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution!

An alternative way to fix this (arguably a better way) would be to modify the impl ErrorDiagnostic for TypeCheckError to check for other suggestions on a TypeCheckError::UnknownIdentifier error

I think I like your way better. Keeps the diagnostic modules' purpose focused on laying out errors in a nice way (without worrying about the contents of the error).

@sharkdp sharkdp merged commit e3f4335 into sharkdp:master Oct 8, 2023
@sharkdp
Copy link
Owner

sharkdp commented Oct 8, 2023

Changes are live on https://numbat.dev: https://numbat.dev/?q=assert((1%20yard%20%3E%201%20meter)%20==%20False)

@eminence eminence deleted the bool_did_you_mean branch October 8, 2023 17:22
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.

2 participants