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

Update #workspaceRoot variables reference #1977

Merged
merged 6 commits into from Oct 16, 2018
Merged

Update #workspaceRoot variables reference #1977

merged 6 commits into from Oct 16, 2018

Conversation

torn4dom4n
Copy link
Contributor

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

The 2 files updated in the PR at the moment are used when developing the extension itself. There are other cases where we are using the $workspaceRoot in the extension's code base. Would you like to update them as well?

  • package.json
  • README
  • util.ts

In util.ts, we replace $workspaceRoot with the actual path. Here, we should support both $workspaceRoot and $workspaceFolder

@ramya-rao-a ramya-rao-a changed the title Update variables reference Update #workspaceRoot variables reference Oct 10, 2018
@ramya-rao-a
Copy link
Contributor

Also, dont forget to register for the Microsoft Hacktoberfest :)

src/util.ts Outdated
if (toolsGopathForWorkspace.startsWith('~')) {
toolsGopathForWorkspace = path.join(os.homedir(), toolsGopathForWorkspace.substr(1));
}
if (toolsGopathForWorkspace && toolsGopathForWorkspace.trim() && !/\${workspaceRoot}/.test(toolsGopathForWorkspace)) {
if (toolsGopathForWorkspace && toolsGopathForWorkspace.trim() && !/\${workspaceFolder}/.test(toolsGopathForWorkspace)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should support both $workspaceRoot and $workspaceFolder here. Not everyone would have updated their settings.

*/
export function resolvePath(inputPath: string, workspaceRoot?: string): string {
export function resolvePath(inputPath: string, workspaceFolder?: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

The resolvePath function was already supporting both $workspaceRoot and $workspaceFolder. We want to retain that. So, please revert the changes done to resolvePath function

@ramya-rao-a ramya-rao-a merged commit cb1869c into microsoft:master Oct 16, 2018
@torn4dom4n
Copy link
Contributor Author

Sorry @ramya-rao-a I didn't read it. I see you did everything 😆.

@ramya-rao-a
Copy link
Contributor

No problem :) I was going through all the PRs and wrapping them up to be ready for the next update

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.

2 participants