Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Fix remote path bug (#2010) #3108

Merged
merged 2 commits into from
Mar 19, 2020
Merged

Fix remote path bug (#2010) #3108

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
116d3ed
Fix remote path bug
quoctruong Mar 17, 2020
aae9fb4
Update src/debugAdapter/goDebug.ts
quoctruong Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions src/debugAdapter/goDebug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,16 @@ function logError(...args: any[]) {
logger.error(logArgsToString(args));
}

function findPathSeparator(filePath: string) {
return filePath.includes('/') ? '/' : '\\';
}

function normalizePath(filePath: string) {
if (process.platform === 'win32') {
const pathSeparator = findPathSeparator(filePath);
filePath = path.normalize(filePath);
// Normalize will replace everything with backslash on Windows.
filePath = filePath.replace(/\\/g, pathSeparator);
return fixDriveCasingInWindows(filePath);
}
return filePath;
Expand Down Expand Up @@ -754,13 +761,6 @@ class GoDebugSession extends LoggingDebugSession {
log('InitializeResponse');
}

protected findPathSeperator(filePath: string) {
if (/^(\w:[\\/]|\\\\)/.test(filePath)) {
return '\\';
}
return filePath.includes('/') ? '/' : '\\';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a possibility that the path has both / and ? The new change selects / in that case while the old findPathSeparator picks \ if it matches the above pattern (in line 758). Wonder if that would be potentially a problem.

}

protected launchRequest(response: DebugProtocol.LaunchResponse, args: LaunchRequestArguments): void {
if (!args.program) {
this.sendErrorResponse(
Expand Down Expand Up @@ -835,10 +835,13 @@ class GoDebugSession extends LoggingDebugSession {
if (this.delve.remotePath.length === 0) {
return this.convertClientPathToDebugger(filePath);
}
// When the filePath has a different path separator
// than the local path separator (cross-compilation),
// the split and join logic won't work.
// See github.com/microsoft/vscode-go/issues/2010.
quoctruong marked this conversation as resolved.
Show resolved Hide resolved
filePath = filePath.replace(/\/|\\/g, this.remotePathSeparator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if it's doable to write some unit tests to check these path manipulation functions.

return filePath
.replace(this.delve.program, this.delve.remotePath)
.split(this.localPathSeparator)
.join(this.remotePathSeparator);
.replace(this.delve.program.replace(/\/|\\/g, this.remotePathSeparator), this.delve.remotePath);
}

protected toLocalPath(pathToConvert: string): string {
Expand Down Expand Up @@ -1392,8 +1395,8 @@ class GoDebugSession extends LoggingDebugSession {
}

if (args.remotePath.length > 0) {
this.localPathSeparator = this.findPathSeperator(localPath);
this.remotePathSeparator = this.findPathSeperator(args.remotePath);
this.localPathSeparator = findPathSeparator(localPath);
this.remotePathSeparator = findPathSeparator(args.remotePath);

const llist = localPath.split(/\/|\\/).reverse();
const rlist = args.remotePath.split(/\/|\\/).reverse();
Expand Down