Only override paths when file actually exists within the cwd/remoteRoot #350
Conversation
| @@ -293,36 +293,44 @@ class RubyDebugSession extends DebugSession { | |||
| return localPath.replace(/\\/g, '/'); | |||
| } | |||
|
|
|||
| protected convertClientPathToDebugger(localPath: string): string { | |||
There was a problem hiding this comment.
I found that most of this code was formatted with tabs while the rest of the file was using spaces so I fixed that up too!
|
Looks like the AppVeyor build is red, but looking at some recently merged PRs the builds had the same failure so unrelated to my changes. Looks like it does not like the |
| @@ -60,7 +60,7 @@ function filter(symbols, query, matcher) { | |||
| .uniq() | |||
| .value(); | |||
| } | |||
| module.exports = class Locate { | |||
| export class Locate { | |||
There was a problem hiding this comment.
Changed this to get the build green, looks like it did the trick!
| @@ -1,6 +1,6 @@ | |||
| import * as vscode from 'vscode'; | |||
| import { ExtensionContext, SymbolKind, SymbolInformation } from 'vscode'; | |||
| import * as Locate from '../locate/locate'; | |||
| import { Locate } from '../locate/locate'; | |||
There was a problem hiding this comment.
Changed this to get the build green, looks like it did the trick!
|
Awesome, thanks! |
|
@wingrunr21 what are the plans for a release? is there currently a time frame? |
|
I'll tag one later tonight and @rebornix can publish it |
|
@wingrunr21 @rebornix any idea when we can expect a release? we have some scenarios that this fix will help improve the teams debugging experience for and it would be great if I could just get them to upgrade. |
|
Sorry, I'll cut and tag tonight. Got sidetracked 👎 |
|
No problems at all! |
This fix improves the ability to debug when running inside of containers and enhances the ability to debug installed gems that do not exist within the workspace root.
Right now I have a hack that will symlink my system installed gems path within my workspace under
tmp/bundlewhich I then overlay with a gem cache volume, I then use that same path as my GEM_HOME within the container so that I am able to step into gems, this is required because currently all paths are blindly prefixed with the cwd/remoteRoot.This change means that if the file exists under say ~/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0, it will leave the path alone and not perform the path prefixing upon converting to and from client/debugger paths. Then when running my container I can ensure the in container gems are installed to the same path and stepping into gems just works.
Interestingly it appears that when setting breakpoints that the full path is not as important and if the last few path segments match it will still work but I made the change to
convertClientPathToDebuggerfor consistency.I plan to look at if we can have the same flexibility when mapping back to the client side which would mean not needing to override GEM_HOME within the container further decreasing the barrier to entry, but for now this fix gets the ball rolling.