kadircet added a comment.
I think it makes more sense to expose semantical information in HoverInfo(like
Name, Scope, Definition, etc), rather than formatted strings, and let this be
serialized by the users of ClangdServer (as in D61497
<https://reviews.llvm.org/D61497>). This is what I'll perform with that patch
anyway, since it aims to provide consumers of ClangdServer with sementical
information. So it is up to you whether to change the layering or keep it as it
is in this patch.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:853
+ Hover R;
+ switch (HoverContentFormat) {
+ case MarkupKind::Plaintext:
----------------
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.
================
Comment at: clang-tools-extra/clangd/Protocol.cpp:707
+ else
+ return false;
+ return true;
----------------
Maybe also vlog/elog the unknown kind? So that we can easily catch new
additions to spec.
================
Comment at: clang-tools-extra/clangd/Protocol.h:359
+enum class MarkupKind {
+ Plaintext,
+ Markdown,
----------------
This is mentioned as `PlainText` in the specs
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61601/new/
https://reviews.llvm.org/D61601
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
