Skip to content

Add support for an lsp language client to specify which capabilities they support.#48685

Merged
CyrusNajmabadi merged 3 commits intodotnet:masterfrom
CyrusNajmabadi:lspCapabilities
Oct 18, 2020
Merged

Add support for an lsp language client to specify which capabilities they support.#48685
CyrusNajmabadi merged 3 commits intodotnet:masterfrom
CyrusNajmabadi:lspCapabilities

Conversation

@CyrusNajmabadi
Copy link
Contributor

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner October 16, 2020 17:50
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@CyrusNajmabadi CyrusNajmabadi force-pushed the lspCapabilities branch 2 times, most recently from 45a8bfd to d050ac7 Compare October 16, 2020 18:00
Copy link
Member

@dibarbet dibarbet left a comment

Choose a reason for hiding this comment

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

other than the xaml initialize, everything else looks good

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@dibarbet dibarbet left a comment

Choose a reason for hiding this comment

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

lgtm, just need to update the tests (maybe we don't even need that test)

@ghost
Copy link

ghost commented Oct 16, 2020

Hello @CyrusNajmabadi!

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.

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

@CyrusNajmabadi
Copy link
Contributor Author

I can try that!

@CyrusNajmabadi CyrusNajmabadi merged commit 7201e75 into dotnet:master Oct 18, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the lspCapabilities branch October 18, 2020 05:53
@ghost ghost added this to the Next milestone Oct 18, 2020
public async Task<InitializeResult> InitializeAsync(InitializeParams initializeParams, CancellationToken cancellationToken)
public Task<InitializeResult> InitializeAsync(InitializeParams initializeParams, CancellationToken cancellationToken)
{
Contract.ThrowIfTrue(_clientCapabilities != null, $"{nameof(InitializeAsync)} called multiple times");
Copy link
Member

Choose a reason for hiding this comment

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

Fun fact: Our code does not live up to this contract check :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait... how?

Copy link
Member

Choose a reason for hiding this comment

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

We initialize _clientCapabilities in the constructor 🤦‍♂️

I'm fixing it :)

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in #48747

@allisonchou allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants