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 all commits
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
26 changes: 14 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,12 @@ class GoDebugSession extends LoggingDebugSession {
if (this.delve.remotePath.length === 0) {
return this.convertClientPathToDebugger(filePath);
}
// The filePath may have a different path separator than the localPath
// So, update it to use the same separator as the remote path to ease
// in replacing the local path in it with remote path
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 +1394,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