Skip to content

Conversation

@psfinaki
Copy link
Contributor

fixes #7080

So this address the final set of cases with let binding tooltips, like this one:
image

@psfinaki psfinaki requested a review from dsyme July 11, 2022 15:34
@psfinaki
Copy link
Contributor Author

@auduchinok please also take a look, for some reason cannot request a review from you.

do
let fu$$nc x = ()
()
""">]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really prefer putting the test case source code in the attribute. It's not the end of the world though.

@auduchinok
Copy link
Member

auduchinok commented Jul 11, 2022

@psfinaki Could FSharpMemberOrFunctionOrValue.CurriedParameterGroups also use this info? FSharpSymbol API is what editor tools use when they work with symbols directly.

It could probably be good to add some tests for CurriedParameterGroups and DeclaringEntity too?

@auduchinok
Copy link
Member

auduchinok commented Jul 11, 2022

@auduchinok please also take a look

Done. :)

for some reason cannot request a review from you.

It's likely because I'm not in the dotnet organization, but feel free to ping me, I'm happy to take a look or to help where possible.

@dsyme
Copy link
Contributor

dsyme commented Jul 11, 2022

@psfinaki Could FSharpMemberOrFunctionOrValue.CurriedParameterGroups also use this info? FSharpSymbol API is what editor tools use when they work with symbols directly.
It could probably be good to add some tests for CurriedParameterGroups and #13429 (comment) too?

I think do this in a separate PR - this is partly a learning exercise for @psfinaki and will be good to get the specific improvement in the bag.

I don't think there will be any downsides to doing it but might be tricky to make sure it doesn't have other effects.

@psfinaki
Copy link
Contributor Author

@KevinRansom @auduchinok alright, so I'll merge this and will do a followup for your suggestions.

@psfinaki psfinaki merged commit 9df7080 into dotnet:main Jul 11, 2022
@psfinaki psfinaki deleted the evi branch July 11, 2022 17:57
@cartermp
Copy link
Contributor

Oh @psfinaki great work! This was such a nag in tools for a long time, glad you figured it out 🙂

Another thing to try out here would be manual testing with the Signature Help feature when calling a function. That feature relies on having parameters and parameter names and making them available in the tooltips.

@psfinaki
Copy link
Contributor Author

Thanks @cartermp :)
Can you point me out to some place to get started with this Signature Help feature?

@auduchinok
Copy link
Member

@psfinaki It's in SignatureHelp.fs. Btw, it has examples of how CurriedParameterGroups property is used in features.

@cartermp
Copy link
Contributor

@psfinaki let me know if you have any troubles with it. I can hopefully remember some context on why some of the weird stuff is the way it is

@psfinaki
Copy link
Contributor Author

@auduchinok @cartermp thank you both :)

@auduchinok
Copy link
Member

@psfinaki Hi, are you still going to work on CurriedParameterGroups?

@psfinaki
Copy link
Contributor Author

psfinaki commented Dec 9, 2024

@auduchinok not right now for sure, this has fallen out of my radar :)

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.

Intellisense tooltip for local functions should show argument names

5 participants