Skip to content

Migrate VSCode to using EditorServices as a standalone, and building that way #1252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 24, 2018

Conversation

rjmholt
Copy link
Contributor

@rjmholt rjmholt commented Mar 31, 2018

Combined with PSES #647, this moves all the building and starting of EditorServices into EditorServices and out of the VSCode extension. Because I changed a bunch of paths, I've tried to make them all as configurable and single-set as possible, but I'm pretty sure there's more work to do here. Just need more eyes at the moment.

src/process.ts Outdated
@@ -40,7 +41,8 @@ export class PowerShellProcess {
const startScriptPath =
path.resolve(
__dirname,
"../../scripts/Start-EditorServices.ps1");
this.bundledModulesPath,
"../scripts/Start-EditorServices.ps1");
Copy link
Member

Choose a reason for hiding this comment

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

So I thought we were just going to leave this to what it was before and then have the build script copy the Start-EditorServices.ps1 and move it under ../../..scripts/Start-EditorServices.ps1

Copy link
Member

Choose a reason for hiding this comment

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

this change moves it under:

modules/scripts/Start-EditorServices.ps1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Debugger shows the runtime value set here to be "C:\Users\roholt\Documents\Dev\PowerShellEditorServices\scripts\Start-EditorServices.ps1" on my machine.

I've got it running against PSES #647.

But could be a fluke. Wanna check it works for you too?

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM! I can do that sanity check you mentioned.

@TylerLeonhardt
Copy link
Member

We just need to make sure we merge in the PSES one first 😃

@rjmholt rjmholt merged commit deec351 into PowerShell:master Apr 24, 2018
@rjmholt rjmholt deleted the change-startpses-path branch December 11, 2019 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants