Fix host process picker when session init'd. #437
Conversation
Also fix the "initialConfigurations" attach setting to use the host process picker and set a default runspaceId.
rkeithhill
changed the title
| @@ -105,7 +114,7 @@ export class PickPSHostProcessFeature implements IFeature { | |||
|
|
|||
| if (this.waitingForClientToken) { | |||
| this.clearWaitingToken(); | |||
| return this.pickPSHostProcess(); | |||
There was a problem hiding this comment.
Shouldn't this still be called so that the menu will be displayed when the connection is finally established?
There was a problem hiding this comment.
It will display the UI but the resulting pick is dropped on the floor AFAICT. As I understand it, the whole process operates like so:
- User picks attach to in debug panel
- Dbg config for attach has
${command.pickPSHostProcess}configured to get theprocessIdvalue - That command is invoked and in turn, activates PowerShell since it is registered as activation event
- When the command completes, VSCode uses the returned
stringas theprocessIdvalue. If the command does not return a string, the adapter gets sent "undefined" forprocessId.
So what I think we need is a synchronization primitive (like *ResetEvent / CriticalSection) so that the command can "wait w/timeout" on the setLanguageClient to signal that it has been initialized.
BTW in the process of debugging this I found a bug (two actually) in VSCode :-) microsoft/vscode#6569 (comment)
There was a problem hiding this comment.
BTW if we want to punt on this for 0.9.0, I could return an error message when PowerShell hasn't been started along the lines of "You must start a PowerShell session before using attach to PowerShell host process".
There was a problem hiding this comment.
OK so reading up some on JavaScript - no need for sync primitives due to single-threaded nature of JavaScript/node. Hmm...
There was a problem hiding this comment.
Yep you shouldn't need to worry about thread synchronization in JavaScript. If this is good enough for now I'll merge it and then poke around with it a bit.
There was a problem hiding this comment.
Yes, go ahead and merge it and see what you think. Hopefully, you can figure out a better way to handle the PowerShell not initialized case.
Tell user to try again after PowerShell has fully initialized. This is not an ideal user experience but it is better than the current experience.
daviwil
changed the title
|
Thanks Keith! |
Also fix the "initialConfigurations" attach setting to use the host process picker and set a default runspaceId.