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.

Sign up for GitHub

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

Jump to bottom

Add to and update TS external access to avoid AbstractProject #49085

Merged
5 commits merged into from
Nov 11, 2020
Merged

Add to and update TS external access to avoid AbstractProject #49085

5 commits merged into from
Nov 11, 2020

Conversation

uniqueiniquity
Copy link
Contributor

In https://dev.azure.com/devdiv/DevDiv/_git/TypeScript-VS/pullrequest/282633, I'm working to remove TS's usage of AbstractProject. Here, we add the necessary external access APIs for this migration.

@uniqueiniquity uniqueiniquity requested a review from a team as a code owner October 30, 2020 20:45
@uniqueiniquity
Copy link
Contributor Author

cc: @jasonmalinowski

@jasonmalinowski jasonmalinowski self-assigned this Nov 2, 2020
jasonmalinowski
jasonmalinowski approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Code changes are good but would still like the two questions about VSTypeScriptContainedDocument answered if possible.

...TypeScript/Api/VSTypeScriptVisualStudioProjectWrapper.LSPContainedDocumentServiceProvider.cs

private sealed class VirtualDocumentPropertiesService : DocumentPropertiesService
{
private const string _lspClientName = "TypeScript";
Copy link
Member

Choose a reason for hiding this comment

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

Is your use of LspClientName here new?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is. This is to keep documents that correspond to script block contents (i.e. "virtual documents") from having their diagnostics published to the error list table control in a manner other than through the lsp publish diagnostics notifications, as Razor does with their generated .cs files.

Copy link
Member

Choose a reason for hiding this comment

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

Alright just going to tag @CyrusNajmabadi and @dibarbet so they know that's happening and it's not just Razor using this mechanism anymore.

@jasonmalinowski
Copy link
Member

Windows_Desktop_Release_64 queue failed due to #49275.

@ghost
Copy link

ghost commented Nov 10, 2020

Hello @jasonmalinowski!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

ghost
ghost approved these changes Nov 10, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@ghost ghost merged commit 2ba9b98 into dotnet:master Nov 11, 2020
@ghost ghost added this to the Next milestone Nov 11, 2020
@allisonchou allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonmalinowski jasonmalinowski jasonmalinowski approved these changes

Assignees

@jasonmalinowski jasonmalinowski

Labels
Area-IDE auto-merge
Projects
None yet
Milestone
16.9.P2
Development

Successfully merging this pull request may close these issues.
4 participants
@uniqueiniquity @jasonmalinowski @allisonchou @Dotnet-GitSync-Bot