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

Fixes #623 Expand ~ to os.homedir() in gopath #768

Merged
merged 5 commits into from
Feb 6, 2017

Conversation

ramya-rao-a
Copy link
Contributor

No description provided.

src/goPath.ts Outdated
if (workspaceRoot) {
inputPath = inputPath.replace(/\${workspaceRoot}/g, workspaceRoot);
}
return (process.platform === 'win32' || !inputPath.startsWith('~')) ? inputPath : path.join(os.homedir(), inputPath.substr(1));
Copy link
Member

Choose a reason for hiding this comment

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

Why not resolve ~ on windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because ~ being the home dir is a POSIX thing. Doesn't apply to Windows.

But I see your point, if there was a workspace setting for go path using ~ checked in to source control, then the same workspace should work on Windows machine.

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Windows has it too now - ~ is expanded (only in powershell), and os.homedir() returns the right thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

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

Looks good, one nit, I would switch the condition around (return inputPath.startsWith ...)

@ramya-rao-a
Copy link
Contributor Author

Sure mr. nitpicker :) Will do.

@ramya-rao-a ramya-rao-a merged commit ab35627 into microsoft:master Feb 6, 2017
@ramya-rao-a ramya-rao-a deleted the homedir branch February 6, 2017 21:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants