[PATCH] D79157: [clangd] Render code complete documentation as plaintext/markdown.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 30 02:44:29 PDT 2020


kadircet marked an inline comment as done.
kadircet added a comment.

thanks, mostly LG apart from dropping of white-space only comments.



================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:377
+      auto SetDoc = [&](llvm::StringRef Doc) {
+        if (!Doc.trim().empty()) {
+          Completion.Documentation.emplace();
----------------
why do we ignore whitespace only comments now ?

nit: early exit;
```
if(Doc.trim().empty())
  return;
...
```


================
Comment at: clang-tools-extra/clangd/Hover.h:83
 // Try to infer structure of a documentation comment (e.g. line breaks).
+// FIXME: move to another file so CodeComplete doesn't depend on Hover.
 void parseDocumentation(llvm::StringRef Input, markup::Document &Output);
----------------
yeah was about to say that :D what about formattedstring.h with a different name like `parseRaw` ?


================
Comment at: clang-tools-extra/clangd/Protocol.cpp:317
+          for (const auto &Format : *DocumentationFormat) {
+            if (fromJSON(Format, K)) {
+              R.CompletionDocumentationFormat = K;
----------------
why not just `if (fromJson(Format, R.CompletionDocFormat)` ?


================
Comment at: clang-tools-extra/clangd/Protocol.cpp:347
         for (const auto &Format : *ContentFormat) {
-          MarkupKind K = MarkupKind::PlainText;
           if (fromJSON(Format, K)) {
             R.HoverContentFormat = K;
----------------
i know it is not related to this change, but you've touched it :P so same as above, maybe just `fromJson(Format, R.HoverContentFormat)`


================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:835
       /* Multi-line
          block comment
       */
----------------
no action needed here(apart from maybe filing a bug), but this one looks like a hard-break to me, as the line below is longer in width. why is parseDocumentation not preserving it?

It might also be the case that i am just wrong, as I didn't check the line break heuristics :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79157





More information about the cfe-commits mailing list