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

Symbol Signature text API #15275

Merged
merged 4 commits into from
Jun 5, 2023
Merged

Symbol Signature text API #15275

merged 4 commits into from
Jun 5, 2023

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented May 30, 2023

Related to #13215 and #14170

I want to add a new API to FSharpMemberOrFunctionOrValue to return the "signature text" via the NicePrint API. This would be useful for editors to provide a light bulb kind of action to add a member or binding to an existing signature file.

I'm not quite sure yet about the name or exact API but I'm very happy with the initial results of this. It is quite accurate and has a lot of details right.

For example:

let f (a : string) (b : char) = fun (c : float) -> 2.0

gives for f

val f: a: string -> b: char -> c: float -> float

Or

type State =
    {
        Files : string list
    }

    static member Empty : State = { Files = [] }

gives for Empty

static member Empty: State

Let me know what you think.

@T-Gro
Copy link
Member

T-Gro commented May 31, 2023

Do you have a real use case in mind already?
For the editor tooling now (closest coming to my mind for this feature are mouseover tooltips), TaggedText is preferred over a plain string - makes more sense given requirements for coloring or for example hyperlinks.

@nojaf
Copy link
Contributor Author

nojaf commented May 31, 2023

Do you have a real use case in mind already?

Adding a member or binding to an existing signature file. Or updating an outdated one.

@vzarytovskii
Copy link
Member

Do you have a real use case in mind already?

Adding a member or binding to an existing signature file. Or updating an outdated one.

Thats sounds nice. We should pilot it in VS as method lens actions (not entirely sure what is the proper name for those).

Cc @psfinaki

@nojaf nojaf force-pushed the signature-symbol-api branch from 4a97c41 to 9857373 Compare June 1, 2023 07:01
@psfinaki
Copy link
Member

psfinaki commented Jun 5, 2023

Might be useful for signature hints. Will look at this in the upcoming days - thanks!

@nojaf
Copy link
Contributor Author

nojaf commented Jun 5, 2023

This API is working out very well for me in Telplin. (Proof of concept in nojaf/telplin#77).

What Telplin does, is traverse the untyped tree to generate a signature counterpart.
Each time it needs typed information, it fetches this for the relevant Symbol.
Invokes the GetValSignatureText call to get the signature-related text.
It parses this and converts it to the Fantomas tree model and inserts it in the signature tree.

The DisplayEnvironment for the Symbol is much better than what FSharpCheckFileResults,GenerateSignature starts with, and since open statements are just copied over from the untyped tree, this works out beautifully. Any details that are missed by GetValSignatureText call (like #15267 for example) can be updated from the untyped tree information. These short-term workarounds should of course be fixed eventually in NicePrint, but it allows for a convenient escape hatch.

As mentioned in the OP, I have incremental editor scenarios in mind as well for this.
VSCode would nearly instantly benefit from this API to add their light bubble action thing.

To summarize, can we have this, please?

@nojaf nojaf marked this pull request as ready for review June 5, 2023 13:01
@nojaf nojaf requested a review from a team as a code owner June 5, 2023 13:01
Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

src/Compiler/Symbols/Symbols.fsi Outdated Show resolved Hide resolved
@psfinaki psfinaki enabled auto-merge (squash) June 5, 2023 14:00
auto-merge was automatically disabled June 5, 2023 14:34

Head branch was pushed to by a user without write access

@psfinaki psfinaki enabled auto-merge (squash) June 5, 2023 14:59
@psfinaki psfinaki merged commit 7abc6b5 into dotnet:main Jun 5, 2023
@nojaf
Copy link
Contributor Author

nojaf commented Jun 5, 2023

Thanks all!

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.

5 participants