[PATCH] D61601: [clangd] Represent Hover result using FormattedString

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 7 01:34:08 PDT 2019


ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:853
+                          Hover R;
+                          switch (HoverContentFormat) {
+                          case MarkupKind::Plaintext:
----------------
kadircet wrote:
> Why not move `R.contents.kind` and `R.range` assignment out of the switch statement. and perform return after the switch. That way you can get rid of the `llvm_unreachable`(we would still get warnings for that switch-statement if someone adds a new `MarkupKind` but doesn't update that statement.
> 
> And hopefully we should fallback to `PlainText` if editor doesn't support any markupkinds known to us.
Done, we do want `llvm_unreachable` at the end, just in case a new `MarkupKind` gets added.

> And hopefully we should fallback to PlainText if editor doesn't support any markupkinds known to us.
This is handled by the code parsing initialization options and it actually falls back to plaintext.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61601





More information about the cfe-commits mailing list