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.

Sign up for GitHub

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

Jump to bottom

Function signature tooltips include attributes for arguments that make the tooltip unreadable #858

Open
kMutagene opened this issue Oct 27, 2023 · 5 comments
Open

Function signature tooltips include attributes for arguments that make the tooltip unreadable #858

kMutagene opened this issue Oct 27, 2023 · 5 comments

Comments

@kMutagene
Copy link
Contributor

kMutagene commented Oct 27, 2023 •
edited
Loading

I am not sure if this is the correct place to raise an issue like this, because it also affects both tooltips in VS and polyglot notebooks

Plotly.NET makes quite heavy use of attributes for method arguments to improve C# interop. A typical method looks like this:

[<Extension>]
static member Point
    (
        x: seq<#IConvertible>,
        y: seq<#IConvertible>,
        [<Optional; DefaultParameterValue(null)>] ?Name: string,
        [<Optional; DefaultParameterValue(null)>] ?ShowLegend: bool,
        [<Optional; DefaultParameterValue(null)>] ?Opacity: float,
        [<Optional; DefaultParameterValue(null)>] ?MultiOpacity: seq<float>,
        [<Optional; DefaultParameterValue(null)>] ?Text: #IConvertible,

       <many more arguments here>

    ) = <body here>

in the past, tooltips for these methods looked like this:

image

with the latest fsdocs version, they include the Optional and DefaultParameterValue Attributes, even prefixed with their full namespaces:

image

This is really unfortunate because they are now completely unusable. This also happens in Ionide and Visual studio:

image

image

So i suspect something bigger out of the scope of this lib being at play. Might this be related to this lib updating the FCS version with > 19.0.0? Is there another place to raise this issue? This is impacting the usability of Plotly.NET heavily.
@nojaf
Copy link
Collaborator

nojaf commented Oct 27, 2023

This rings a bell. I believe this is a change with NicePrint over in the F# compiler.
I remember some time ago that I wanted to include parameter attributes in the signatures files.
This impacts:

image

I don't think this is problematic in itself. It accurately reflects what the code contains.
The formatting maybe could improve over at the compiler side? I'm not sure, maybe it is fine and what the ID does with the result should improve. I also think Ionide does its own thing with tooltips.

And I'm pretty sure that fsharp-formatting might also drop the ball in the tooltip department.

@kMutagene
Copy link
Contributor Author

I don't think this is problematic in itself. It accurately reflects what the code contains.

While technically correct, i am not sure if i agree here generally. Maybe my use case for these tooltips is non-standard, but i generally want to know the names and types of the arguments - which are really hidden in visual noise, even when formatted better like in your screenshot. It also might be that the methods of Plotly.NET just have an unusual amount of arguments (sometimes > 100), so it might be an extreme case where tooltips reach their limit anyways.

I do not want to sound too complain-y here though - at the end of the day if this the way to display tooltips i'll adapt, but i think for this specific use case the update made things worse.

If you could point me to the code where tooltips are generated, i could add a flag to the tool that trims these from the input if that is a possibility. Should be quite easy to remove with a regex, as everything is guarded by [<>]

As an aside - the API docs tooltips for methods look 100x better than this IMO. could we just display that as tooltip (without the param descriptions)?

image

@nojaf
Copy link
Collaborator

nojaf commented Oct 30, 2023

I think extending the DisplayEnvironment (in https://github.com/dotnet/fsharp/blob/1e78d396a1397009fe52d609660376f0ce5c5d52/src/Compiler/Symbols/SymbolHelpers.fs#L546-L551) and only add the attributes when our new flag is enabled (Somewhere in https://github.com/dotnet/fsharp/blob/4934588ae8dfac3de2e8ce58198601edbfddfd08/src/Compiler/Checking/NicePrint.fs#L1047) makes the most sense. We would disable it for tooltips.

We can pair on this sometime if you want.

@kMutagene
Copy link
Contributor Author

We can pair on this sometime if you want.

Sure, do you intend to stream this? I am not sure if i will have time in the next week(s), but i'll make sure to reach out once i am free

@nojaf
Copy link
Collaborator

nojaf commented Nov 3, 2023

Sure, do you intend to stream this?

We could but that is really up to you. I wasn't specifically proposing it with that in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nojaf @kMutagene