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

Fix: Inline parameter hints for inner bindings #14506

Conversation

rosskuehl
Copy link
Contributor

@rosskuehl rosskuehl commented Dec 21, 2022

Fixes fsharp/issues/14501

By keeping track of long identifier ending positions for function calls, we can avoid recapturing parameters from the outer scope while traversing the syntax tree for hints.

There is likely a more efficient/direct way to patch this behavior, but this is a relatively simple change and appears to work as expected 👨‍🍳👍

Screenshots

Before
Screenshot 2022-12-22 at 3 17 41 PM

After
Screenshot 2022-12-22 at 3 13 55 PM

@rosskuehl rosskuehl requested a review from a team as a code owner December 21, 2022 20:40
@T-Gro
Copy link
Member

T-Gro commented Dec 22, 2022

In PR's related to visual editor features, can we please have screenshot(s) ?
Ideally before & after, at least the 'after' one.

Seeing the test, I think it also makes sense (separate to this PR!) to add common Fsharp.Core functions/arguments to a blacklist ;; "mapping" from Seq/List/Array.map being one of them.

@rosskuehl
Copy link
Contributor Author

Thanks for the feedback @T-Gro! I added before & after screenshots to the description.

@baronfel
Copy link
Member

baronfel commented Dec 22, 2022

I think it also makes sense (separate to this PR!) to add common Fsharp.Core functions/arguments to a blacklist ;; "mapping" from Seq/List/Array.map being one of them

We did just that in FSAC (after looking at the Rust-Analyzer decisions here) and tried to capture our heuristics:

https://github.com/fsharp/FsAutoComplete/blob/d74332594a29b8bae412032a2e75a4818f7a633f/src/FsAutoComplete.Core/InlayHints.fs#L337-L354

@vzarytovskii vzarytovskii enabled auto-merge (squash) December 27, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Inline parameter hints are incorrectly shown for inner bindings
4 participants