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

Filter out some useless parameter name inlay hints #932

Merged
merged 5 commits into from
Apr 27, 2022

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Apr 27, 2022

Fixes #924 and parts of ionide/ionide-vscode-fsharp#1693.

We apply a few heuristics to parameter name hint generation specifically now:

  • the parameter has to have an utterable name
  • the parameter must not be one of a handful of well-known names that aren't especially useful (NOTE: could use some help here with suggestions, primarily from FSharp.Core and/or popular libraries)
  • the parameter must have a name of length > 2 (filters out lots of math variables and dummy parameters)
  • the parameter name must not overlap the name of the user-provided text. i.e. if the parameter name is format and the user provided a let binding named format, do not show the hint.

Additionally, we truncate all hints to 30 characters to prevent editors from chopping them up into multiple sub-hints. This is an arbitrary choice and means that we need to introduce a separate field for the text to insert for type hints, since they may be truncated.

Other heuristics that have not yet been done:

  • actually identify unary functions and always ignore their parameters
  • tweak prefix check to understand camelCase/snake_case conventions and only accept prefixes where that is the case

@Krzysztof-Cieslak
Copy link
Member

Should we filter out the case where the name of the passed value is the same as the name of the parameter?

image

@baronfel baronfel force-pushed the filter-code-lenses branch from 3c654ae to e175323 Compare April 27, 2022 14:28
@baronfel
Copy link
Contributor Author

yep! that's the 'overlap' condition mentioned in the description.

@Krzysztof-Cieslak
Copy link
Member

I can't read

@baronfel
Copy link
Contributor Author

I need to add test cases to show these cases explicitly, then it should become very clear by example.

@baronfel
Copy link
Contributor Author

Quick screenshot of some samples from local testing, the red underlines are cases where we no longer show type hints

image

@baronfel baronfel marked this pull request as ready for review April 27, 2022 17:56
@baronfel baronfel merged commit 75f809d into ionide:main Apr 27, 2022
@baronfel baronfel deleted the filter-code-lenses branch April 27, 2022 19:07
@baronfel
Copy link
Contributor Author

@Booksbaum with this setup in place now, if you think of good ways to expand on some of the heuristics you mentioned in the issue it should be very easy to suggest/test them.

@baronfel baronfel changed the title Filter out useless parameter name inlay hints Filter out some useless parameter name inlay hints Apr 27, 2022
@Booksbaum
Copy link
Contributor

Will do.

Filtering out short params and same param&argument probably already covers most intrusive cases.

More useful suggestions probably come up, when it's in actual use (-> ionide).

One enhancement I can think of:
Hide not just argument same as param or starts with param, but also ends with param:

let f range = ()
let exactRange = ()

f exactRange
//     ^^^^^

&

type Data = {
  Name: string
}
let f name = ()
let data = { Name = "..." }

f data.Name
//     ^^^^

hm...I think I'm going to take a closer look over the new few days and play a bit around :)


And: It's great to see the Server Test stuff in action for something else than just CodeFixes 😄

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.

Hide some Inlay Hints
3 participants