Skip to content

Insert doc comments when typing via LSP#46301

Merged
davidwengier merged 16 commits intodotnet:masterfrom
davidwengier:CommentsAutoCompleteInLSP
Jul 28, 2020
Merged

Insert doc comments when typing via LSP#46301
davidwengier merged 16 commits intodotnet:masterfrom
davidwengier:CommentsAutoCompleteInLSP

Conversation

@davidwengier
Copy link
Copy Markdown
Member

@davidwengier davidwengier commented Jul 24, 2020 •
edited by Pilchie
Loading

Fixes AB#1155667

Adds support to LSP for inserting doc comments during typing, both when typing '///' above a method/class, or when pressing enter within a doc comment.

You'll pretty much have to review commit-by-commit, since the first commit is a straight cut and paste of code into a service without modification. Clean up comes later to make reviewing it easier.

@davidwengier davidwengier requested a review from a team as a code owner July 24, 2020 10:47
@jinujoseph jinujoseph added Area-IDE LSP issues related to the roslyn language server protocol implementation labels Jul 24, 2020
dibarbet
dibarbet reviewed Jul 25, 2020
View reviewed changes
Copy link
Copy Markdown
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.

mostly looks good - if there are any areas in the doc comment service where you made material logic changes instead of mostly copy paste let me know

allisonchou
allisonchou approved these changes Jul 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@allisonchou allisonchou left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 (though I'd probably get another approval in addition just to be sure) All of my comments are nits, although I didn't look too closely at the move to service since I'm assuming it's just code being moved over.

@davidwengier
Copy link
Copy Markdown
Member Author

davidwengier commented Jul 27, 2020

if there are any areas in the doc comment service where you made material logic changes instead of mostly copy paste let me know

The only change of note with the existing code is that in the command handler, two of the operations did an Insert and two did a Replace. Now they all do a Replace so there was some tweaking of the spans to get that to work, but the existing command handler tests all pass without modification so I'm pretty confident :)

@davidwengier
Copy link
Copy Markdown
Member Author

davidwengier commented Jul 27, 2020

All of the PR feedback has been addressed

@davidwengier davidwengier merged commit 61dd560 into dotnet:master Jul 28, 2020
@ghost ghost added this to the Next milestone Jul 28, 2020
@davidwengier davidwengier deleted the CommentsAutoCompleteInLSP branch July 28, 2020 07:52
@RikkiGibson RikkiGibson modified the milestones: Next, 16.8.P2 Aug 11, 2020
@davidwengier davidwengier mentioned this pull request Aug 25, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dibarbet dibarbet dibarbet approved these changes

+1 more reviewer

@allisonchou allisonchou allisonchou approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-IDE LSP issues related to the roslyn language server protocol implementation

Projects

None yet

Milestone

16.8.P2

Development

Successfully merging this pull request may close these issues.

5 participants

@davidwengier @dibarbet @allisonchou @RikkiGibson @jinujoseph