[clang-tools-extra] 54d7db1 - [clangd] Move inserted include from detail -> documentation.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-04-30T19:58:53+02:00
New Revision: 54d7db165d438bf1bc4f13212a0a7bd3e61aae39

URL: https://github.com/llvm/llvm-project/commit/54d7db165d438bf1bc4f13212a0a7bd3e61aae39
DIFF: https://github.com/llvm/llvm-project/commit/54d7db165d438bf1bc4f13212a0a7bd3e61aae39.diff

LOG: [clangd] Move inserted include from detail -> documentation.

Summary: Many clients try to display all the detail inline, with poor results.

See https://github.com/clangd/clangd/issues/284

Reviewers: hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D79106

Added: 
    

Modified: 
    clang-tools-extra/clangd/CodeComplete.cpp
    clang-tools-extra/clangd/FormattedString.cpp
    clang-tools-extra/clangd/FormattedString.h
    clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
    clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index abf9e1fddaf6..4c7f1457518c 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1824,6 +1824,21 @@ bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
   return false;
 }
 
+// FIXME: find a home for this (that can depend on both markup and Protocol).
+static MarkupContent renderDoc(const markup::Document &Doc, MarkupKind Kind) {
+  MarkupContent Result;
+  Result.kind = Kind;
+  switch (Kind) {
+  case MarkupKind::PlainText:
+    Result.value.append(Doc.asPlainText());
+    break;
+  case MarkupKind::Markdown:
+    Result.value.append(Doc.asMarkdown());
+    break;
+  }
+  return Result;
+}
+
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
   CompletionItem LSP;
   const auto *InsertInclude = Includes.empty() ? nullptr : &Includes[0];
@@ -1838,19 +1853,15 @@ CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
                    ? std::string(llvm::formatv("[{0} overloads]", BundleSize))
                    : ReturnType;
   LSP.deprecated = Deprecated;
-  if (InsertInclude)
-    LSP.detail += "\n" + InsertInclude->Header;
-  if (Documentation) {
-    LSP.documentation.emplace();
-    switch (Opts.DocumentationFormat) {
-    case MarkupKind::PlainText:
-      LSP.documentation->value = Documentation->asPlainText();
-      break;
-    case MarkupKind::Markdown:
-      LSP.documentation->value = Documentation->asMarkdown();
-      break;
-    }
-    LSP.documentation->kind = Opts.DocumentationFormat;
+  // Combine header information and documentation in LSP `documentation` field.
+  // This is not quite right semantically, but tends to display well in editors.
+  if (InsertInclude || Documentation) {
+    markup::Document Doc;
+    if (InsertInclude)
+      Doc.addParagraph().appendText("From ").appendCode(InsertInclude->Header);
+    if (Documentation)
+      Doc.append(*Documentation);
+    LSP.documentation = renderDoc(Doc, Opts.DocumentationFormat);
   }
   LSP.sortText = sortText(Score.Total, Name);
   LSP.filterText = Name;

diff  --git a/clang-tools-extra/clangd/FormattedString.cpp b/clang-tools-extra/clangd/FormattedString.cpp
index 96a5290780cb..d24103f0dd61 100644
--- a/clang-tools-extra/clangd/FormattedString.cpp
+++ b/clang-tools-extra/clangd/FormattedString.cpp
@@ -434,6 +434,11 @@ Document &Document::operator=(const Document &Other) {
   return *this;
 }
 
+void Document::append(Document Other) {
+  std::move(Other.Children.begin(), Other.Children.end(),
+            std::back_inserter(Children));
+}
+
 Paragraph &Document::addParagraph() {
   Children.push_back(std::make_unique<Paragraph>());
   return *static_cast<Paragraph *>(Children.back().get());

diff  --git a/clang-tools-extra/clangd/FormattedString.h b/clang-tools-extra/clangd/FormattedString.h
index ccb758f294c9..4cdaf2002ed4 100644
--- a/clang-tools-extra/clangd/FormattedString.h
+++ b/clang-tools-extra/clangd/FormattedString.h
@@ -88,6 +88,8 @@ class Document {
   Document(Document &&) = default;
   Document &operator=(Document &&) = default;
 
+  void append(Document Other);
+
   /// Adds a semantical block that will be separate from others.
   Paragraph &addParagraph();
   /// Inserts a horizontal separator to the document.

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index c9799525de3f..1f30d4314d78 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1665,8 +1665,8 @@ TEST(CompletionTest, Render) {
   EXPECT_EQ(R.insertText, "Foo::x");
   EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText);
   EXPECT_EQ(R.filterText, "x");
-  EXPECT_EQ(R.detail, "int\n\"foo.h\"");
-  EXPECT_EQ(R.documentation->value, "This is x()");
+  EXPECT_EQ(R.detail, "int");
+  EXPECT_EQ(R.documentation->value, "From \"foo.h\"\nThis is x()");
   EXPECT_THAT(R.additionalTextEdits, IsEmpty());
   EXPECT_EQ(R.sortText, sortText(1.0, "x"));
   EXPECT_FALSE(R.deprecated);
@@ -1688,7 +1688,8 @@ TEST(CompletionTest, Render) {
 
   C.BundleSize = 2;
   R = C.render(Opts);
-  EXPECT_EQ(R.detail, "[2 overloads]\n\"foo.h\"");
+  EXPECT_EQ(R.detail, "[2 overloads]");
+  EXPECT_EQ(R.documentation->value, "From \"foo.h\"\nThis is x()");
 
   C.Deprecated = true;
   R = C.render(Opts);
@@ -1696,7 +1697,7 @@ TEST(CompletionTest, Render) {
 
   Opts.DocumentationFormat = MarkupKind::Markdown;
   R = C.render(Opts);
-  EXPECT_EQ(R.documentation->value, "This is `x()`");
+  EXPECT_EQ(R.documentation->value, "From `\"foo.h\"`  \nThis is `x()`");
 }
 
 TEST(CompletionTest, IgnoreRecoveryResults) {

diff  --git a/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp b/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
index d3fa1aa8c0a7..6202f930b504 100644
--- a/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -240,6 +240,17 @@ TEST(Document, Ruler) {
   EXPECT_EQ(D.asPlainText(), "foo\n\nfoo");
 }
 
+TEST(Document, Append) {
+  Document D;
+  D.addParagraph().appendText("foo");
+  D.addRuler();
+  Document E;
+  E.addRuler();
+  E.addParagraph().appendText("bar");
+  D.append(std::move(E));
+  EXPECT_EQ(D.asMarkdown(), "foo  \n\n---\nbar");
+}
+
 TEST(Document, Heading) {
   Document D;
   D.addHeading(1).appendText("foo");