-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add suggestions when misspelling a record field #1710
Conversation
cli/tests/snapshot/snapshots/snapshot__eval_stderr_record_access_suggestion.ncl.snap
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
id: ident, | ||
field_names: record | ||
.field_names(RecordOpKind::IgnoreEmptyOpt), | ||
operator: String::from("(.$)"), |
There was a problem hiding this 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 where is that .$
coming from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good catch. In fact the operator wasn't shown before this PR (basically the notes
were created but never used - it's unrelated to this PR, it's just that I needed to add the suggestion to the notes, so I realized, and it was in fact surprising that the rust compiler haven't yelled at me for unused value until now). So we might have let old names slipped through.
I believe this was the original syntax for dynamic access field, like foo.$x
was the current foo."%{x}"
, because we used to have $
as the interpolation character and we didn't have quoted fields yet, such as foo."bar^baz"
. When we added the later syntax, it felt better to avoid keeping the strange .$
which could just be subsumed by quoted field access + interpolation.
Closes #1708.
This pull request shows suggestions of similar field names when trying to access a non-existent field of a record.
This first implementation uses the Damerau-Levensthein distance. The choice of the algorithm is inspired from the Rust compiler, although they use a more involved and hand-written variant with corrections around substrings: we just take one from the seemingly most popular string similarity library on crates.io,
strsim
.More specifically, we try to find either a perfect match ignoring cases, or the field with the highest similarity with the user input, as long as it's above some threshold (to avoid suggesting things that have nothing to do with the input). The threshold was chosen rather arbitrarily, by trial and error: it seems that choosing a higher threshold didn't work that well for very small words (4 letters word), which can have low similarity even with an edit distance of 1 or 2.