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

Don't show parameter name hints for basic types #14548

Closed
wants to merge 1 commit into from

Conversation

psfinaki
Copy link
Member

@psfinaki psfinaki commented Jan 5, 2023

closes #14448

Was:
image

Now:
image

@psfinaki psfinaki requested a review from a team as a code owner January 5, 2023 16:27
@@ -82,6 +82,12 @@ module InlineParameterNameHints =
symbolUse.IsFromUse
&& symbol.DisplayName <> "(::)"

let isTooBasic (symbol: FSharpSymbol) =
Copy link
Member

@vzarytovskii vzarytovskii Jan 5, 2023

Choose a reason for hiding this comment

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

Can this be renamed to something like isBuiltinKnownType or isBuiltinKnownSymbol?

@@ -82,6 +82,12 @@ module InlineParameterNameHints =
symbolUse.IsFromUse
&& symbol.DisplayName <> "(::)"

let isTooBasic (symbol: FSharpSymbol) =
let denyList = ["Some"; "Ok"; "Error"]
Copy link
Member

Choose a reason for hiding this comment

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

I would also add things like dict, string, set there.

@@ -82,6 +82,12 @@ module InlineParameterNameHints =
symbolUse.IsFromUse
&& symbol.DisplayName <> "(::)"

let isTooBasic (symbol: FSharpSymbol) =
let denyList = ["Some"; "Ok"; "Error"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be a set. And defined on module level.

@vzarytovskii
Copy link
Member

One thing which bothers me a bit is that knowledge about built-in types is leaking into editor layer.
I am wondering if we can (re)use attributes for it.

@KevinRansom
Copy link
Member

This seems more like it should be a feature similar to the DebuggerView in the framework. Apply an attribute to the type under consideration and have the editor notice the attribute and perform the specified formatting. The current implementation has privileged types for this type of formatting, already vlad has identified some additional types that may benefit from this. Which will cause us to have to update the editor whenever new privileged types are identified.

If we create an attribute similar to debuggerviewattribute perhaps editorformatattribute then third parties will be able to provide specific formatting themselves. Anyway, I am reluctant to create an additional location for privileged types.

@psfinaki
Copy link
Member Author

psfinaki commented Jan 5, 2023

Hmmm my idea was to strip just these two things as very ubiquitous and very easy to strip, before getting some feedback and coming with a more systematic approach. The one with an attribute is a viable one.

@psfinaki
Copy link
Member Author

psfinaki commented Jan 6, 2023

Okay we'll do a more proper implementation, closing this for now then.

@psfinaki psfinaki closed this Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Don't show parameter name hints for some built-in types
4 participants