ionide / ionide-vscode-fsharp

VS Code plugin for F# development
http://ionide.io
MIT License
865 stars 279 forks source link

Tuples mess up inlay parameter hints #1714

Closed olivercoad closed 2 years ago

olivercoad commented 2 years ago

Describe the bug

Inlay hints treat tuple arguments as separate parameters to a function, leading to the following parameters being labelled wrong.

Steps to reproduce

Set "editor.inlayHints.enabled": "on" in vscode settings to view inlay hints

let inlayHintsTest firstParam tupleParam lastParam = ()
inlayHintsTest "firstParam" ("t1", "t2") "lastParam"

Expected behaviour

The tuple ("t1", "t2") should be labelled as a whole as tupleParam and "lastParam" should be labelled correctly.

Screenshots

image

Machine info

Additional context

I wanted to try out inlay hints now that vscode has added offUnlessPressed. It doesn't seem to work though as they aren't visible unless it's set to on. They don't even show up with onUnlessPressed. I'm guessing this is probably a vscode issue rather than an ionide issue though.

baronfel commented 2 years ago

@olivercoad thanks for the detailed report, it really helps!

@Booksbaum can you cover this case with testing in your FSAC branch with all the inlayHint fixes?

olivercoad commented 2 years ago

nvm about the offUnlessPressed bit not working. Just realised I still had "FSharp.inlayHints.enabled": false, in my global vscode settings, which I'd forgotten about because it seems to not disable them when editor.inlayHints.enabled is "on" or unset. But I guess it does still disable them with other values of editor.InlayHints.enabled. After removing FSharp.inlayHints.enabled from my global settings, offUnlessPressed works great. And the hover tooltip to change the setting is nice.

tldr: don't set FSharp.inlayHints.enabled anymore, use editor.inlayHints.enabled instead.

Booksbaum commented 2 years ago

can you cover this case with testing in your FSAC branch with all the inlayHint fixes?

It's unfortunately not a trivial case: Tuple itself might be named (like here in issue) -> name for whole tuple. Or Items might be named -> name for items.

And in code we currently collect flattened args (-> no distinction between normal curried or tupled param)

Further complication: what to do when tuple items are named, but tuple value is provided as param?:
ParamHintTuple
(last line: f2 t)
(Probably either show no param hint or a combined param hint ((val1,val2)= or similar))

All doable (func.CurriedParameterGroups has that info for params, and AstTraversal can be adjusted to detect tupled vs normal) -- but that requires some work.



I'm not doing this for fsharp/FsAutoComplete#943.

But there are some additional enhancements for inlay hints I want to try afterwards. I can attempt to fix this then.