Skip to content
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

[Discussion] [Live Share] Restrict language services to local files #1264

Merged
merged 4 commits into from
Apr 13, 2018

Conversation

lostintangent
Copy link
Contributor

In preparation for Visual Studio Live Share adding support for "guests" to receive remote language services for PowerShell, this PR simply updates the current DocumentSelector to be limited to file and untitled (unsaved) files. This way, when someone has the PowerShell extension installed, and joins a Live Share session (where files use the vsls: scheme), their language services will be entirely derived from the remote/host side, which provides a more accurate and project-wide experience (guests in Live Share don't have local file access to the project they're collaborating with).

If someone joins a PowerShell project using Live Share, and doesn't have the PowerShell extension installed, then they will automatically receive language services from the host (which is awesome! 🎉), so this PR is simply an optimization for the case where collaborating developers both have the PowerShell extension installed. Additionally, this wouldn't impact the "local" PowerShell development experience.

Note: As an example, the TypeScript/JavaScript language services that come in-box with VS Code already have this scheme restriction, and so this PR replicates that behavior.

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'm super excited for this feature!

@rjmholt
Copy link
Contributor

rjmholt commented Apr 12, 2018

@SteveL-MSFT want to loop you in here — possible security considerations?

@SteveL-MSFT
Copy link
Member

@rjmholt it doesn't seem that there's a security concern here as if I understand this change correctly, without this the guest invited to a live share would have access to the remote files already and also local files, but this change restricts the share session to just the remote files.

@lostintangent
Copy link
Contributor Author

@SteveL-MSFT That is correct!

Copy link
Contributor

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

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

Ok, I've fully parsed this now. Looks like a really good change! Particularly happy that we've gotten rid of a few hard-coded strings as well.

@TylerLeonhardt TylerLeonhardt merged commit 9a363c7 into PowerShell:master Apr 13, 2018
@TylerLeonhardt
Copy link
Member

Thanks for contributing @lostintangent!!

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.

4 participants