Launch and Attach commands should respect the cwd parameter #24
Conversation
TikiTDO
changed the title
|
@ko1 Any chance you could take a look at this PR? I'd really like to use this extension for my project, but it will not work without these changes. |
| if (config.cwd) { | ||
| // Ensure we are in the requested working directory | ||
| const cd_command = "cd " + config.cwd; | ||
| outputTerminal.sendText(cd_command); |
There was a problem hiding this comment.
Does it work correctly if cwd is relative path?
There was a problem hiding this comment.
It would need to be a relative path from the workspace root.
There was a problem hiding this comment.
Is it safe to start repeatedly?
There was a problem hiding this comment.
I tried with cwd: "foo" (there is ${workspaceFolder}/foo directory) and it doesn't work on line get_sock_list() (ENOENT), so my question "Does it work correctly if cwd is relative path?" is "NO" and my real question "does it work on repeated launch with relative path cwd" is YES because this code is not reachable with a relative path.
(FYI: "cwd": "${workspaceFolder}/foo" works fine)
For better solution maybe we need to use File.expand_path() in Ruby to solve the relative path to absolute path, but I'm not sure how to do it on nodejs.
I'm okay to merge this commit. Do you want to care about this File.expand_path issue or can I merge it without it?
There was a problem hiding this comment.
I will fix it today. Just stuck in meetings for a few hours. Will get to it by the end of the day my time.
There was a problem hiding this comment.
In other words, can I merge this PR and close it?
There was a problem hiding this comment.
Yes, this PR can be merged and closed.
I wanted to see if you're interested in documenting the above behavior. If you are then I can open another PR for that.
There was a problem hiding this comment.
Thank you. I can't understand your point yet. To use with rvm this debugger needs wrapper script, you mean?
There was a problem hiding this comment.
Correct. I just want to know if you would like me to make a PR describing how I did this in the README.md.
There was a problem hiding this comment.
Before adding it to the doc, I want to know why this wrapper is needed for rvm.
But this discussion should be done in other issue/PR, maybe.
| @@ -44,7 +44,7 @@ function export_breakpoints(context: vscode.ExtensionContext) { | |||
| // outputChannel.appendLine(JSON.stringify(bp)); | |||
| const start_line = bp.location.range.start.line; | |||
| const path = bp.location.uri.path; | |||
| bp_lines = bp_lines + "break " + path + ":" + start_line + "\n" | |||
| bp_lines = bp_lines + "break " + path + ":" + start_line + "\n"; | |||
There was a problem hiding this comment.
It's not strictly needed, but the rest of the code had it so my formatter added it in. I can change my formatter to remove all ;
There was a problem hiding this comment.
If you'd like you can create a config file called .prettierrc.js with the following options:
module.exports = {
semi: false,
}
Then you can install this plugin: https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode
It works a lot like rubocop, formatting your code to idiomatic typescript.
There was a problem hiding this comment.
Note: You would also need to change this eslint rule: "@typescript-eslint/semi": "warn", -> "@typescript-eslint/semi": "off",
|
Thank you I'll merge it when the question is solved. |
Both the launch and attach commands currently ignores the
cwdparameter, which causes the debugger to fail in monorepos that have theGemfilein a sub-directory. This PR ensures that thecwdparameter is applied everywhere it's necessary onlaunch.Example:
If my rails server is in
${workspaceFolder}/backend, with myGemfilein${workspaceFolder}/backend/Gemfile, then I would like to runbundle exec rdbgin${workspaceFolder}/backendas opposed to${workspaceFolder}