proposal: do not show "did you mean X" if the edit distance is too great

[iacore]

Hello, Gleam developers

I was just trying out Gleam at https://tour.gleam.run/ and saw this confusing message.

error: Unknown variable
  ┌─ /src/main.gleam:4:3
  │
4 │   println("My lucky number is:")
  │   ^^^^^^^ Did you mean `Nil`?

The name `println` is not in scope here.

It took me awhile to figure out what "Did you mean Nil?" meant.

To my memory, Rust won't show the same error message if the identifier is too far from any name in scope.

Here is the error message without the confusing part:

error: Unknown variable
  ┌─ /src/main.gleam:4:3
  │
4 │   println("My lucky number is:")
  │   ^^^^^^^

The name `println` is not in scope here.

which i argue is better.

[lpil]

Good idea!

[Monik2002]

Is there any help I can do here?

[lpil]

You can pick up this work if you want for sure!

[Monik2002]

Do I have to make changes only in error.rs file or do I have to change all the .snap files as well?

[lpil]

Update the error.rs file and then you can update the snapshots using cargo insta, the snapshot testing tool we use.

[tubedude]

This proved to be a bit harder than expected but I came up with the solution here: https://github.com/tubedude/gleam/tree/better_recommendation.

I borrowed from Rust compiler but had to adjust to fit for some cases I felt the current solution made more sense. For example, unknown_record_field (test here) should indeed suggest inner in place of unknown but I was only able to achieve this by returning if there was only one suggestion.

[iacore]

I borrowed from Rust compiler but had to adjust to fit for some cases I felt the current solution made more sense. For example, unknown_record_field (test here) should indeed suggest inner in place of unknown but I was only able to achieve this by returning if there was only one suggestion.

rustc's error messages really set the bar high... maybe it's better not have the special case (when there is only one option) so the typo fixer behaves more consistently.

[tubedude]

Yeah! rustc and elm's error messages are very good examples. I was not running clippy before committing and only saw the various corrections after doing the PR. I'll work to adjust that.

As for the special case. I tend to agree on not having it, but the example in the test is so compelling (it suggests inner – which is correct – in place of unknown) that I thought it would be a shame to change it. I actually would like to understand what is the situation that generates this single suggestion as all the others have more items on the options list.

[iacore]

As for the special case. I tend to agree on not having it, but the example in the test is so compelling (it suggests inner – which is correct – in place of unknown) that I thought it would be a shame to change it. I actually would like to understand what is the situation that generates this single suggestion as all the others have more items on the options list.

I would say no, especially since the list of fields is shown below in the error message.

pub type Box(a) { Box(inner: a, inner_b: a) }
pub fn main(box: Box(Int)) { box.unknown }

error: Unknown record field
  ┌─ /src/main.gleam:4:33
  │
4 │ pub fn main(box: Box(Int)) { box.unknown }
  │                                 ^^^^^^^^ Did you mean `inner`?

The value being accessed has this type:

    Box(Int)

It has these fields:

    .inner
    .inner_b

[tubedude]

Makes sense! I removed it. Some 6 tests were impacted.

[lpil]

Please revert that change, thank you.