15.3 rollup - #1581
Conversation
Sorry, something went wrong.
|
digeff can you give this a spin with PZ tomorrow, ping me on what to test. |
Sorry, something went wrong.
note: adding support for the legacy webkit debugger requires changes to the IVsDebugLaunchTargetProvider. So I decided against that for now.
| // marked completed. | ||
| this.currentCommand = this.commandQueue.Take(); | ||
| if (this.currentCommand != null) | ||
| if (this.commandQueue.TryTake(out var command, Timeout.Infinite) && command != null) |
There was a problem hiding this comment.
What's the reason for this change? I don't see any change in semantics.
Sorry, something went wrong.
There was a problem hiding this comment.
Take throws an exception when the collection is marked as completed, where TryTake returns false. There was a racecondition I saw a couple of times between Dispose and Take being called.
Sorry, something went wrong.
| // We check the registry to see if any parameters for the node.exe invocation have been specified (like "--inspect"), and append them if we find them. | ||
| var nodeParams = NodejsProjectLauncher.CheckForRegistrySpecifiedNodeParams(); | ||
| if (!string.IsNullOrEmpty(nodeParams)) | ||
| { |
There was a problem hiding this comment.
Assuming setupInstance.GetInstallationPath() can return a path with spaces, and this is all just being concated with args into one long string, does this require any quoting?
Sorry, something went wrong.
There was a problem hiding this comment.
And possibly all the other paths we handle here....
digeff Thoughts?
Sorry, something went wrong.
| $@"Microsoft\VisualStudio\NodeAdapter\{visualStudioInstallationInstanceID}\out\src\nodeDebug.js"""); | ||
| } | ||
|
|
||
| var target = vsDebugTargetInfo.bstrExe; |
There was a problem hiding this comment.
Can the user not specify the working directory to use in the NTVS property pages for the project?
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, this is the open folder scenario though. We don't use the project properties there.
Sorry, something went wrong.
Bill Ticehurst (billti)
left a comment
There was a problem hiding this comment.
Look into the comments I left. If no issue, looks good to me.
Sorry, something went wrong.
Various fixes we want to get into Update 3 for VS 2017