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

Implement mouseover tooltips for keywords #13956

Merged
merged 16 commits into from
Sep 22, 2022

Conversation

T-Gro
Copy link
Member

@T-Gro T-Gro commented Sep 22, 2022

This PR implements mouseover tooltips for keywords in Visual Studio.
This already works in FsAutoComplete (Ionide) and this new VS implementation uses the same data source, provided by Compiler Services.

Here is a visual example.
image

REMARK:
In order to test, I had to rebase this branch on top of recent "packages-update" of all Roslyn and Visual Studio packages.
Without it, "build.cmd -testVS" was failing in latest Int Preview version of visual studio.

@T-Gro T-Gro linked an issue Sep 22, 2022 that may be closed by this pull request
@dnfadmin
Copy link

dnfadmin commented Sep 22, 2022

CLA assistant check
All CLA requirements met.

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.

So I'd probably remove the icons at all, I don't see any particular value in having them whereas they take space :)

Otherwise - awesome!

src/Compiler/Service/ServiceLexing.fs Outdated Show resolved Hide resolved
@T-Gro
Copy link
Member Author

T-Gro commented Sep 22, 2022

So I'd probably remove the icons at all, I don't see any particular value in having them whereas they take space :)

Otherwise - awesome!

We do use icons in other tooltips, always fitting the respective token (property, class, method,... have their own icons).
Having one for keywords keeps the behavior consistent.

@psfinaki
Copy link
Member

So I'd probably remove the icons at all, I don't see any particular value in having them whereas they take space :)
Otherwise - awesome!

We do use icons in other tooltips, always fitting the respective token (property, class, method,... have their own icons). Having one for keywords keeps the behavior consistent.

Sure. I don't have a strong opinion here :)

@vzarytovskii vzarytovskii enabled auto-merge (squash) September 22, 2022 15:11
@T-Gro T-Gro disabled auto-merge September 22, 2022 16:18
@T-Gro T-Gro merged commit 0ee2932 into main Sep 22, 2022
@T-Gro T-Gro deleted the 12062-implement-tooltips-for-keywords branch September 26, 2022 07:39
@vzarytovskii vzarytovskii mentioned this pull request Sep 26, 2022
@@ -31,6 +31,8 @@ type public ToolTipElementData =
ParamName: string option
}

static member Create: layout: TaggedText[] * xml: FSharpXmlDoc * ?typeMapping: TaggedText[] list * ?paramName: string * ?remarks: TaggedText[] -> ToolTipElementData
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be internal please? I think it is only used from FSharpCheckerResults.fs? Thanks

@@ -344,6 +344,9 @@ module FSharpKeywords =
/// Keywords paired with their descriptions. Used in completion and quick info.
val KeywordsWithDescription: (string * string) list

/// A lookup from keywords to their descriptions
val KeywordsDescriptionLookup: System.Collections.Generic.IDictionary<string, string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be a string -> string option method please, GetKeywordDescription? Rather than revealing an IDictionary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go to definition on external dependency/metadata is broken in 17.4 Implement tooltips for keywords
5 participants