-
Notifications
You must be signed in to change notification settings - Fork 831
use xml doc from signature if non present in impl #14920
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
Conversation
nojaf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
|
Excellent finding! |
It was also working for simple let bindings. /// Some Sig Doc on myC
val myC: intin your .fsi file, that also worked. |
psfinaki
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job, @dawedawe, thanks a lot - looking forward to leverage this on the daily basis!
|
Yes, Visual Studio seems to do something extra. So there are more cases in which VS shows the sig comments. ActivePatterns, Partial Active Patterns and class types for example don't work currently in VS, too. |
|
@dawedawe No worries, this is useful at least from the perspective of testing all this stuff. If you want to do some extra checks, feel free to, if not - that's also fine, we can merge this as-is I think. |
F# part of VS is open, you can check in the vsintegration directory in the repo |
Uh, thanks for the pointer. That's great. I really should have seen this earlier 😞 |
|
@dawedawe The active pattern case is interesting: I expect the documentation to be available on the individual cases while its provided on the whole pattern, and it's not. The last screenshots show that R# tooltip somehow got in instead of our F# ones, and it's not expected. We should fix it. |
That feels related to the range of an active pattern not including the |
|
@auduchinok @nojaf |
|
Thanks for all the screenshots :) As for the range fix, I guess it can be done separately as well - or here, I'm fine with both as long as it's tested. |
|
I'd prefer a separate one for that. |
|
Alright! Thanks, merging. |
The problem is FCS doesn't return the documentation for the active pattern when you hovering a case, it's not related to parens/ranges. It might be a Rider-specific issue. |













Fixes #14906
Works and tests now for