[PATCH] D67720: [clangd] Add semantic selection to ClangdLSPServer.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 added a comment.

I am not sure adding client capability is useful here. I have not used the client capability for selectionRange anywhere and I think we can remove it. 
WDYT ?



================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1131
+    Callback<std::vector<SelectionRange>> Reply) {
+  if (Params.positions.size() != 1) {
+    elog("{0} positions provided to SelectionRange. Supports exactly one "
----------------
ilya-biryukov wrote:
> hokein wrote:
> > maybe add an `assert(!Params.positions.empty())`. I think we should not run into this case.
> But `Params` comes to clangd over LSP, right?
> That means `assert` can fire in case of bad inputs over LSP to clangd.
> Bad inputs over LSP should never crash clangd.
Yes this comes from the client and can be a bad input. We should just return error and not crash in such case.


================
Comment at: clang-tools-extra/clangd/Protocol.h:1241
+llvm::json::Value toJSON(const SelectionRange &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SelectionRange &);
+
----------------
hokein wrote:
> does this operator get used in this patch?
No. Removed it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67720/new/

https://reviews.llvm.org/D67720