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

Fix inner help completion #740

Merged
merged 3 commits into from
Sep 17, 2018

Conversation

rjmholt
Copy link
Contributor

@rjmholt rjmholt commented Sep 4, 2018

Fixes PowerShell/vscode-powershell#1509.

I'm not sure what the added lines ?? part of the comment based help request handler was trying to achieve, but I don't think it was correct. So I've removed that.

My second commit refactors the method body a bit to make it a bit simpler.

Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

As long as this still works for the "complete doc comment help before the function" case, it LGTM.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@rjmholt
Copy link
Contributor Author

rjmholt commented Sep 5, 2018

@rkeithhill I did try the help completions both before and after the function for this PR. Are there any other cases I should try? We should also test on Windows

@rjmholt
Copy link
Contributor Author

rjmholt commented Sep 17, 2018

Ok I've tested now!

@darksidemilk
Copy link

Is there any sort of expected release date for when this fix will be available to end users?
Or is there a way I could compile the extension myself to get this functionality back before then?

@TylerLeonhardt
Copy link
Member

We're shooting for later this week. You could also grab the VSIX (not the v2 one - v2 is PSReadLine) here:
https://ci.appveyor.com/project/PowerShell/vscode-powershell/build/artifacts

@rjmholt
Copy link
Contributor Author

rjmholt commented Sep 18, 2018

Before you grab that VSIX, let me make sure it's been rebuilt. Will post again here when it's done.

@rjmholt
Copy link
Contributor Author

rjmholt commented Sep 19, 2018

@darksidemilk
Copy link

Is there some change to how it works, because at first I was going to report that it is not working. But then I noticed the release note about asynchronous processing and noted that the function I was adding documentation too has quite a few parameters. SO I added the ## to it and waited, went to go try it on a different machine came back and the help info was there at last. So It's kinda working but I feel like it shouldn't take 5-10 minutes to create it. Maybe there's something I'm doing that is causing it to hang?

@darksidemilk
Copy link

Is there some change to how it works, because at first I was going to report that it is not working. But then I noticed the release note about asynchronous processing and noted that the function I was adding documentation too has quite a few parameters. SO I added the ## to it and waited, went to go try it on a different machine came back and the help info was there at last. So It's kinda working but I feel like it shouldn't take 5-10 minutes to create it. Maybe there's something I'm doing that is causing it to hang?

Did a little more testing. It does work as expected when I installed the preview version on a different machine in a different script. So maybe this can hang when there are too many parameters defined in a function or something? In my particular situation it's in some functions I wrote before I truly understood how to use the param () section and was putting all the parameters and their default values there. Which I now know isn't the best approach and will just edit those functions a bit. But I imagine its possible to have 10+ parameters in a function with default values in them. Perhaps that is a condition that can cause help completion to hang?

@rjmholt rjmholt deleted the fix-inner-help-completion branch December 12, 2018 06:02
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.

5 participants