microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.51k stars 29.39k forks source link

Incorrectly flag property "cwd" as not allowed for node attach in launch.json. #31465

Closed cantonese84 closed 4 years ago

cantonese84 commented 7 years ago

Steps to Reproduce:

  1. Add configuration to launch.json for node attach
  2. Type "cwd" as property.

VSCode would say "Property cwd is not allowed". Actually, the property is required for node attach to work properly with Meteor 1.6-beta. Without it, break points would not be recognized. The property should be allowed and intellisense should offer it.

weinand commented 7 years ago

What is the semantics of setting the "cwd" for attach? Since node.js is not launched I cannot set its cwd.

cantonese84 commented 7 years ago

Here is the launch.json that works with Meteor 1.6-beta. Without cwd, all breakpoints would show up grayed out, saying "Breakpoint ignored because generated codes not found (source map problem?)."

    {
      "type": "node",
      "request": "attach",
      "name": "Server: attach",
      "port": 9229,
      "sourceMaps": true,
      "protocol": "inspector",
      "restart": true,
      "cwd": "${workspaceRoot}/"
    }
weinand commented 7 years ago

@roblourens are you actually using "cwd" in attach mode?

roblourens commented 7 years ago

Yes, for resolving magic webpack:// and meteor:// sourcemap paths.

https://github.com/Microsoft/vscode-node-debug2/blob/master/src/nodeDebugAdapter.ts#L23 and https://github.com/Microsoft/vscode-node-debug2/blob/master/src/nodeDebugAdapter.ts#L212

weinand commented 7 years ago

Ok, then I will make "cwd" a proper attribute for "attach" (and update doc).

weinand commented 7 years ago

@roblourens when trying to document cwd in the release notes and node debugging doc, it occurred to me that using cwd for sourcemap paths is a bit strange.

Currently we use this description (which is clearly for the launch case): "launch the program to debug in this directory."

I have a hard time to adapt this explanation so that it works for both the "launch" and "attach" case.

It looks like that you are using cwd in your sourceMapPathOverrides because it happens to have the same value as workspaceRoot (and because the extension ensures that cwd is set to workspaceRoot when not explicitly set by the user).

But you are not really using cwd in the same semantics as the cwd used in a "launch" config. What you probably need is a "workspaceRoot" or "projectRoot", correct?

I suggest that we introduce a new optional attribute "workspaceRoot" or "projectRoot" or "sourceMapRoot" (or whatever you suggest)

And in the extension we make sure, that this attribute always has a value (like cwd).

I will reopen this for September.

roblourens commented 7 years ago

Yes, I guess that's actually what I need.

brucejo75 commented 7 years ago

Hi, in 1.17.1both cwd and sourceMapPathOverrides work today. When you resolve this issue will you provide indicators in the launch.json editor?

e.g. "cwd no longer supported for attach, maybe you want to define projectRoot"

Thanks

brucejo75 commented 7 years ago

Another comment:

I notice that sourceMapPathOverrides has a useful characteristic:

    "sourceMapPathOverrides": {
      "meteor://💻app/packages/*": "${workspaceFolder}/packages/*",  // Example: "meteor://💻app/main.ts" -> "c:/code/main.ts"
      "meteor://💻app/*": "${workspaceFolder}/meris/*"  // Example: "meteor://💻app/main.ts" -> "c:/code/main.ts"
    },

Works as I intend. In Meteor I can place my packages in a location outside of my project via an environment variable METEOR_PACKAGE_DIRS.

I believe that being able to define different paths for multiple sub-paths is a feature worth keeping. Currently in the 1.17.1 UI I get green sqigglies and a rollover error for the property sourceMapPathOverrides:

Property sourceMapPathOverrides is not allowed

Will this property continue to exist? I am hoping so.

Thanks

roblourens commented 7 years ago

I guess I need to add that property to node-debug too... it's not going anywhere soon :)

iegik commented 5 years ago

It is a bug, because of documentation: https://code.visualstudio.com/docs/editor/debugging#_launch-configurations

Error:

Attribute 'cwd' does not exists ('...')

My suggestion:

Name attribute workingDirectory (like in Linux-like pwd command abbreviation).

{
      "type": "node",
      "request": "launch",
      "name": "yarn run start",
      "workingDirectory": "${workspaceFolder}/packages/some-package",
      "runtimeExecutable": "yarn",
      "runtimeArgs": [
            "run",
            "start"
      ]
},
weinand commented 4 years ago

Since we got a new JavaScript debug extension "js-debug", and the old "node-debug" has become obsolete, we do not plan to work on this issue.