Initial draft of LSP pull diagnostics for documents. by CyrusNajmabadi · Pull Request #47914 · dotnet/roslyn

CyrusNajmabadi · 2020-09-21T21:48:39Z

No description provided.

davidwengier · 2020-09-21T21:52:53Z

You also need to add a method to this nonsense here: https://github.com/dotnet/roslyn/blob/master/src/VisualStudio/Core/Def/Implementation/LanguageClient/InProcLanguageServer.cs#L142

And you probably need to update the Initialize handler to tell the client that we support pull diagnostics, here: https://github.com/dotnet/roslyn/blob/master/src/Features/LanguageServer/Protocol/Handler/Initialize/InitializeHandler.cs#L43

dibarbet

I'm a little unsure of how the diagnostic service events are interacting with the lsp requests here, some explanation might be good

src/Features/LanguageServer/Protocol/Handler/Diagnostics/DocumentPullDiagnosticHandler.cs

davidwengier

Makes sense in general

src/Features/LanguageServer/Protocol/Handler/Diagnostics/DocumentPullDiagnosticHandler.cs

…stics

src/Features/LanguageServer/Protocol/Handler/Diagnostics/DocumentPullDiagnosticHandler.cs

src/VisualStudio/Core/Test.Next/Services/LspDiagnosticsTests.cs

dibarbet

mainly lgtm!
one request, a few comments
Could you verify in a local razor file that other (non-diagnostic) c# LSP features work (e.g., go to definition) when the LSP razor editor flag is set? I assume diagnostics won't until @TanayParikh completes the work for pull diagnostics, but I want to make sure that my assumption that the rest of the c# features work w/out the RazorLanguageClient is true

src/Features/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs

src/Features/LanguageServer/Protocol/Handler/Diagnostics/WorkspacePullDiagnosticHandler.cs

src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs

src/VisualStudio/Core/Def/Implementation/LanguageClient/InProcLanguageServer.cs

src/VisualStudio/Core/Def/Implementation/LanguageClient/RazorLanguageClient.cs

dibarbet

missed a couple comments in the first pass

src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs

src/Features/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs

ghost · 2020-09-30T22:30:09Z

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.

ghost

Auto-approval

…stics" This reverts commit 3911ee0, reversing changes made to 60ae83d.

Revert "Merge pull request #47914 from CyrusNajmabadi/lspDiagnostics"

…spDiagnostics"" This reverts commit 02a9e20.

…nostics"" This reverts commit 02a9e20.

Initial document pull diagnostics for LSP

3ad3a32

Spelling

5390b2a

dibarbet reviewed Sep 21, 2020

View reviewed changes

davidwengier reviewed Sep 21, 2020

View reviewed changes

CyrusNajmabadi added 2 commits September 21, 2020 16:02

Remove NRT

2fa1afc

Move to helper extension.

e4b3ab8

CyrusNajmabadi force-pushed the lspDiagnostics branch from b29ae00 to e4b3ab8 Compare September 21, 2020 23:09

CyrusNajmabadi added 10 commits September 21, 2020 16:13

Simplify

30e2a0d

Use immutable arrays

67ada52

Rename type for clarity

128b100

Make error list strongly typed

af7d01a

Merge branch 'master-vs-deps' into lspDiagnostics

028f3ab

Merge remote-tracking branch 'upstream/master-vs-deps' into lspDiagno…

777b46b

…stics

REvert

437a3b2

Use new

02b3dfb

rename

70b092f

Disable razor and push diagnostics.

53327bd

jinujoseph added the Area-IDE label Sep 25, 2020

CyrusNajmabadi added 3 commits September 28, 2020 12:23

Merge remote-tracking branch 'upstream/master-vs-deps' into lspDiagno…

86106a9

…stics

Remove code.

6ba372c

Use vs diagnostic.

e068c43

dibarbet reviewed Sep 28, 2020

View reviewed changes

src/Features/LanguageServer/Protocol/Handler/Diagnostics/DocumentPullDiagnosticHandler.cs Outdated Show resolved Hide resolved

src/Features/LanguageServer/Protocol/Handler/Diagnostics/DocumentPullDiagnosticHandler.cs Outdated Show resolved Hide resolved

CyrusNajmabadi added 2 commits September 28, 2020 14:31

Workspace pull diagnostics

b0ceef6

Add base

88033eb

This was referenced Sep 28, 2020

Test failures via timeline in Roslyn jaredpar/runfo#37

Closed

Test failures via timeline in Roslyn jaredpar/runfo#38

Closed

CyrusNajmabadi added 3 commits September 28, 2020 14:49

in progress

431ae7c

Workspace diagnostics

9ad7c2c

Cleanup

9f46084

CyrusNajmabadi marked this pull request as ready for review September 30, 2020 01:32

CyrusNajmabadi requested a review from a team as a code owner September 30, 2020 01:32

Add comment

9b023ad

davidwengier reviewed Sep 30, 2020

View reviewed changes

src/VisualStudio/Core/Test.Next/Services/LspDiagnosticsTests.cs Show resolved Hide resolved

davidwengier approved these changes Sep 30, 2020

View reviewed changes

Remove old tests

ae0bb80

dibarbet approved these changes Sep 30, 2020

View reviewed changes

dibarbet reviewed Sep 30, 2020

View reviewed changes

src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs Outdated Show resolved Hide resolved

src/Features/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs Outdated Show resolved Hide resolved

CyrusNajmabadi added 9 commits September 29, 2020 21:01

useusing

18fc408

Add docs

a53bd60

Use single queue

3f0f527

Remove loc string

64390c6

Move helper

154d679

Comments

c9ad86f

Simplify

6ed9cf3

Docs

db7fcf4

Restore

9785055

CyrusNajmabadi force-pushed the lspDiagnostics branch from 56236e3 to 9785055 Compare September 30, 2020 22:12

CyrusNajmabadi added the auto-merge label Sep 30, 2020

ghost approved these changes Sep 30, 2020

View reviewed changes

ghost merged commit 3911ee0 into dotnet:master-vs-deps Oct 1, 2020

ghost added this to the Next milestone Oct 1, 2020

dibarbet added a commit to dibarbet/roslyn that referenced this pull request Oct 1, 2020

Revert "Merge pull request dotnet#47914 from CyrusNajmabadi/lspDiagno…

02a9e20

…stics" This reverts commit 3911ee0, reversing changes made to 60ae83d.

ghost pushed a commit that referenced this pull request Oct 2, 2020

Merge pull request #48228 from dibarbet/revert_pull

98be2b1

Revert "Merge pull request #47914 from CyrusNajmabadi/lspDiagnostics"

CyrusNajmabadi added a commit to CyrusNajmabadi/roslyn that referenced this pull request Oct 5, 2020

Revert "Revert "Merge pull request dotnet#47914 from CyrusNajmabadi/l…

1cadaee

…spDiagnostics"" This reverts commit 02a9e20.

dibarbet pushed a commit that referenced this pull request Oct 7, 2020

Revert "Revert "Merge pull request #47914 from CyrusNajmabadi/lspDiag…

3859b2c

…nostics"" This reverts commit 02a9e20.

Cosifne modified the milestones: Next, 16.9.P1 Oct 12, 2020

CyrusNajmabadi deleted the lspDiagnostics branch April 11, 2021 18:24

This pull request was closed.

Conversation

CyrusNajmabadi commented Sep 21, 2020

Uh oh!

davidwengier commented Sep 21, 2020

Uh oh!

dibarbet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidwengier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dibarbet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dibarbet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ghost commented Sep 30, 2020

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.

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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.