Page MenuHomePhabricator

[clangd][NFC] Simplify handing on methods with no params
ClosedPublic

Authored by njames93 on Jan 22 2021, 3:39 PM.

Details

Summary

Add bind methods handling the case when a method has an empty params interface and when it has no parameters.

Remove ShutdownParams and ExitParams from Protocol, In LSP they aren't defined, instead the methods are defined to have void as the params. This signature now better reflects that.

Diff Detail

Event Timeline

njames93 created this revision.Jan 22 2021, 3:39 PM
njames93 requested review of this revision.Jan 22 2021, 3:39 PM

thanks this LG, but I suppose we can do some more trimming. after the trimming we are introducing a single partial specialization which is luckily more specialized than existing notification-binding specialization, so all should be fine.

clang-tools-extra/clangd/ClangdLSPServer.cpp
262

why do we need this one ?

321

i suppose we can also drop this one after getting rid of InitializedParams?

clang-tools-extra/clangd/ClangdLSPServer.h
95–96

any reasons for keeping InitializedParams ?

clang-tools-extra/clangd/Protocol.h
267–268

and drop this too.

I'm not convinced this is clearer - these methods do in fact return null.
And it adds more duplicative code. Is it necessary?

I'm not convinced this is clearer - these methods do in fact return null.
And it adds more duplicative code. Is it necessary?

Doh, of course I misread - inputs rather than outputs.
Yes, this seems OK if we drop the NoParams variants.
(Though I'm still not sure it's worth the special case...)

njames93 updated this revision to Diff 318942.Jan 25 2021, 3:40 AM
njames93 marked an inline comment as done.

Remove unnecessary specialization.

clang-tools-extra/clangd/ClangdLSPServer.cpp
262

Good point, this one is never actually used now.

clang-tools-extra/clangd/ClangdLSPServer.h
95–96

Although its empty, its defined in the protocol so we should be mirroring it.

clang-tools-extra/clangd/Protocol.h
267–268

See previous comment about mirroring the spec.

sammccall accepted this revision.Jan 25 2021, 9:23 AM
This revision is now accepted and ready to land.Jan 25 2021, 9:23 AM
This revision was automatically updated to reflect the committed changes.