Skip to content

Conversation

david-driscoll
Copy link
Member

  • This makes a new package that is used for both client and server
    • this also decouples the client from the server.
  • Changed out all the method names textDocument/x etc to all be static strings.
    • I feel like there are some more ways we can improve this experience for clients and servers by creating a set of base classes and some generic type magic, similar to how the vscode lsp implementation does... however I do not want to tackle that here.

@david-driscoll
Copy link
Member Author

cc @RLittlesII

@codecov
Copy link

codecov bot commented Nov 30, 2017

Codecov Report

Merging #50 into master will increase coverage by 2.11%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
+ Coverage   66.79%   68.91%   +2.11%     
==========================================
  Files         196      191       -5     
  Lines        2078     2017      -61     
==========================================
+ Hits         1388     1390       +2     
+ Misses        690      627      -63
Impacted Files Coverage Δ
...er/Capabilities/DocumentOnTypeFormattingOptions.cs 66.66% <ø> (ø)
src/Protocol/Models/Registration.cs 100% <ø> (ø)
src/Protocol/Models/Location.cs 100% <ø> (ø)
src/Protocol/Models/MessageActionItem.cs 100% <ø> (ø)
src/Protocol/Models/InitializeError.cs 100% <ø> (ø)
src/Protocol/Models/DocumentLinkParams.cs 100% <ø> (ø)
.../Protocol/Models/DocumentOnTypeFormattingParams.cs 100% <ø> (ø)
src/Protocol/Models/MarkedString.cs 100% <ø> (ø)
...c/Protocol/Server/Capabilities/TextDocumentSync.cs 84.61% <ø> (ø)
src/Protocol/Models/TextDocumentItem.cs 100% <ø> (ø)
... and 175 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ec355f...eb2ff5d. Read the comment docs.

Copy link
Collaborator

@tintoy tintoy left a comment

Choose a reason for hiding this comment

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

Looks fine to me; I agree the namespaces were starting to look a little weird 😁

@RLittlesII
Copy link
Contributor

I would generally agree with your reflective magic as static strings are an eye sore, and further agree it is a separate body of work. The refactoring on this looks good.

@david-driscoll david-driscoll merged commit 61c4255 into master Dec 1, 2017
@david-driscoll david-driscoll deleted the refactor/protocol branch December 1, 2017 00:11
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.

3 participants