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

[sammccall]

Summary

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

[hokein]

clang-tools-extra/clangd/CodeComplete.cpp

1855–1865

#include => From, IIUC discussion from the chat.

maybe worth a comment explaining why we make such a decision, the description seems good enough.

[sammccall]

This needed a nontrivial merge after D79157, because we need to concatenate markup::Documents. Don't think there's anything surprising in there so going ahead, happy to address any comments though.

clang-tools-extra/clangd/CodeComplete.cpp

if (InTopLevelScope(ND))
     return true;
 
   if (const auto *EnumDecl = dyn_cast<clang::EnumDecl>(ND.getDeclContext()))
     return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
 
   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];
   LSP.label = ((InsertInclude && InsertInclude->Insertion)
                    ? Opts.IncludeIndicator.Insert
                    : Opts.IncludeIndicator.NoInsert) +
               (Opts.ShowOrigins ? "[" + llvm::to_string(Origin) + "]" : "") +
               RequiredQualifier + Name + Signature;
 
   LSP.kind = Kind;
   LSP.detail = BundleSize > 1
                    ? std::string(llvm::formatv("[{0} overloads]", BundleSize))
                    : ReturnType;
   LSP.deprecated = Deprecated;
+  // 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)
-    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;
+      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;
   LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name};
   // Merge continuous additionalTextEdits into main edit. The main motivation
   // behind this is to help LSP clients, it seems most of them are confused when
   // they are provided with additionalTextEdits that are consecutive to main
   // edit.
   // Note that we store additional text edits from back to front in a line. That

clang-tools-extra/clangd/FormattedString.h

 class Document {
 public:
   Document() = default;
   Document(const Document &Other) { *this = Other; }
   Document &operator=(const 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.
   void addRuler();
   /// Adds a block of code. This translates to a ``` block in markdown. In plain
   /// text representation, the code block will be surrounded by newlines.
   void addCodeBlock(std::string Code, std::string Language = "cpp");
   /// Heading is a special type of paragraph that will be prepended with \p

clang-tools-extra/clangd/FormattedString.cpp

 
 Document &Document::operator=(const Document &Other) {
   Children.clear();
   for (const auto &C : Other.Children)
     Children.push_back(C->clone());
   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());
 }
 
 void Document::addRuler() { Children.push_back(std::make_unique<Ruler>()); }
 
 void Document::addCodeBlock(std::string Code, std::string Language) {

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

TEST(CompletionTest, Render) {
   Opts.IncludeIndicator.NoInsert = "";
   Opts.EnableSnippets = false;
 
   auto R = C.render(Opts);
   EXPECT_EQ(R.label, "Foo::x(bool) const");
   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);
   EXPECT_EQ(R.score, .5f);
 
   Opts.EnableSnippets = true;
   R = C.render(Opts);
   EXPECT_EQ(R.insertText, "Foo::x(${0:bool})");
   EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet);
 
   Include.Insertion.emplace();
   R = C.render(Opts);
   EXPECT_EQ(R.label, "^Foo::x(bool) const");
   EXPECT_THAT(R.additionalTextEdits, Not(IsEmpty()));
 
   Opts.ShowOrigins = true;
   R = C.render(Opts);
   EXPECT_EQ(R.label, "^[AS]Foo::x(bool) const");
 
   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);
   EXPECT_TRUE(R.deprecated);
 
   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) {
   auto Results = completions(
       R"cpp(
           namespace ns { int NotRecovered() { return 0; } }
           void f() {
             // Sema enters recovery mode first and then normal mode.

clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

TEST(Document, Ruler) {
 
   // Multiple rulers between blocks
   D.addRuler();
   D.addParagraph().appendText("foo");
   EXPECT_EQ(D.asMarkdown(), "foo  \n\n---\nfoo");
   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");
   D.addHeading(2).appendText("bar");
   D.addParagraph().appendText("baz");
   EXPECT_EQ(D.asMarkdown(), "# foo  \n## bar  \nbaz");
   EXPECT_EQ(D.asPlainText(), "foo\nbar\nbaz");
 }