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

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

Merged
Ramya Rao (ramya-rao-a) merged 5 commits into
microsoft:masterfrom
ramya-rao-a:homedir
Feb 6, 2017
Merged

Fixes #623 Expand ~ to os.homedir() in gopath#768
Ramya Rao (ramya-rao-a) merged 5 commits into
microsoft:masterfrom
ramya-rao-a:homedir

Conversation

@ramya-rao-a

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@roblourens roblourens left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor Author

Sure mr. nitpicker :) Will do.

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