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

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54d7db165d43: [clangd] Move inserted include from detail 
-> documentation. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D79106?vs=260962&id=261298#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79106

Files:
  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


Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -240,6 +240,17 @@
   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");
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1665,8 +1665,8 @@
   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 @@
 
   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 @@
 
   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) {
Index: clang-tools-extra/clangd/FormattedString.h
===================================================================
--- clang-tools-extra/clangd/FormattedString.h
+++ clang-tools-extra/clangd/FormattedString.h
@@ -88,6 +88,8 @@
   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.
Index: clang-tools-extra/clangd/FormattedString.cpp
===================================================================
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -434,6 +434,11 @@
   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());
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1824,6 +1824,21 @@
   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 @@
                    ? 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;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits