dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.92k stars 786 forks source link

Make Type Hints less intrusive #14382

Open psfinaki opened 1 year ago

psfinaki commented 1 year ago

So this is quite annoying:

https://user-images.githubusercontent.com/5451366/203621934-99ec33fb-d39d-4c38-9611-eb1684f79d21.mp4

Instead, I'd prefer the behavior like in C# on the right - show the type when we can provide useful information (e.g. when the equal sign is printed).

psfinaki commented 1 year ago

We had a brief session on this with @vzarytovskii - but our best idea so far is just to look if there are any diagnostics within the range, like don't show any hints if there is an error in the code. Seems a bit crude but better than the current situation.

@dsyme @T-Gro @kerams Maybe you have any ideas?

kerams commented 1 year ago

9v7F1SdBMi

Here the typar of b is unsolved. Since incomplete bindings like that seem to be assigned placeholder generic type variables, mayhaps it would be a good rule of thumb not to show generic type hints unless they are solved (which the type of the value a above is)?

T-Gro commented 1 year ago

I would not couple it with diagnostics - there might be diagnostical errors which work just fine with type hints. Instead I would prevent showing useless information => if a type cannot be derived, the hint is not telling much.

psfinaki commented 1 year ago

Well the problem with this is that this info seems to be buried very deep in the symbol and it will take some effort to propagate it properly. But I'll give it a try.

image

Or I just cannot find a better way :D But at least the property IsUnresolved returns false in both cases which was my hope at first.

0101 commented 1 year ago

Could we somehow get the hint to be on the other side of the cursor? I think that would help no only this case but usability in general.

psfinaki commented 1 year ago

Likely yes. I think this is orthogonal to the current issue, so I'll create another one :)

vzarytovskii commented 1 year ago

it will take some effort to propagate it properly

I'm not sure that we should expose a type implementation detail via symbol API.

psfinaki commented 1 year ago

Hmhm we can probably fix at least some cases leveraging the IsSolveAtCompileTime prop available without much effort. Stay tuned.

dsyme commented 1 year ago

My main thoughts (beyond those already discussed here) are that type hints like these should not be on by default. They should show with a hot key. That may be the default that's planned, I'm not sure?

vzarytovskii commented 1 year ago

My main thoughts (beyond those already discussed here) are that type hints like these should not be on by default. They should show with a hot key. That may be the default that's planned, I'm not sure?

Yeah, they're off by default now, and can be enabled in options, toggling via hotkeys (both on press and toggle-based) will be later.

psfinaki commented 1 year ago

Reopening this to also handle type hints in functions:

https://user-images.githubusercontent.com/5451366/204516716-e0db2da1-8b5f-48ba-b652-3d231adda4dc.mp4

psfinaki commented 1 year ago

Unassigning me for now in case someone else wants to play with this as well.