[PATCH] D55363: [clangd] Expose FileStatus in LSP.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 20 05:12:50 PST 2018


ilya-biryukov added inline comments.


================
Comment at: clangd/Protocol.cpp:769
   O.map("fallbackFlags", Opts.fallbackFlags);
+  O.map("fileStatus", Opts.FileStatus);
   return true;
----------------
Ah, there's a non-zero chance of name clash here in case the protocol implements something similar.
Maybe name the JSON field "clangdFileStatus" to be completely sure this won't clash later.


================
Comment at: clangd/Protocol.h:1002
+  /// Details of the state that are worth sufacing to users.
+  std::vector<ShowMessageParams> details;
+};
----------------
As discussed offline, maybe leave out the 'details' field in the first version?
It does not seem to provide much value as of now. Could be totally useful if we showed a fallback compile command used or similar.


================
Comment at: clangd/index/Merge.cpp:137
     S.ReturnType = O.ReturnType;
+  if (S.Type == "")
+    S.Type = O.Type;
----------------
Accidental change?


================
Comment at: unittests/clangd/IndexTests.cpp:222
   R.Origin = SymbolOrigin::Static;
+  R.Type = "expectedType";
 
----------------
Accidental change?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55363





More information about the cfe-commits mailing list