[WIP][clang-doc] Improve the Markdown output

[phosek]

Summary

This change tries to improve the usability of Markdown output, taking
the inspiration from other documentation generators where appropriate.

[paulkirth]

Overall I like the direction of the patch, I know this is WIP, but I've left some small question inline regarding scope and work tracking.

clang-tools-extra/clang-doc/MDGenerator.cpp

23–52

This isn't something I expect to get addressed in this patch, but I feel like I've seen this same pattern of escaping characters repeated throughout the codebase. Maybe we should add a TODO regarding it? I'm also fine with ignoring this.

110–113

Are these TODOs (and the others throughout the patch) something you plan to address in this patch before landing? or are they future improvements? If the latter, maybe we should file an issue on github reference it here?

clang-tools-extra/clang-doc/MDGenerator.cpp

 
 using namespace llvm;
 
 namespace clang {
 namespace doc {
 
 // Markdown generation
 
+static std::string genEscaped(StringRef Name) {
+  std::string Buffer;
+  llvm::raw_string_ostream Stream(Buffer);
+  for (unsigned I = 0, E = Name.size(); I != E; ++I) {
+    unsigned char C = Name[I];
+    switch (C) {
+      case '\\':
+      case '`':
+      case '*':
+      case '_':
+      case '{':
+      case '}':
+      case '[':
+      case ']':
+      case '(':
+      case ')':
+      case '#':
+      case '+':
+      case '-':
+      case '.':
+      case '!':
+        Stream << '\\';
+        LLVM_FALLTHROUGH;
+      default:
+        Stream << C;
+        break;
+    }
+  }
+  return Stream.str();
+}
+
 static std::string genItalic(const Twine &Text) {
   return "*" + Text.str() + "*";
 }
 
 static std::string genEmphasis(const Twine &Text) {
   return "**" + Text.str() + "**";
 }
 
+static std::string genCode(const Twine &Text) {
+  return "`" + Text.str() + "`";
+}
+
 static std::string
 genReferenceList(const llvm::SmallVectorImpl<Reference> &Refs) {
   std::string Buffer;
   llvm::raw_string_ostream Stream(Buffer);
   for (const auto &R : Refs) {
     if (&R != Refs.begin())
       Stream << ", ";
     Stream << R.Name;
   }
   return Stream.str();
 }
 
-static void writeLine(const Twine &Text, raw_ostream &OS) {
-  OS << Text << "\n\n";
+static void writeHeader(const Twine &Text, unsigned int Num, raw_ostream &OS, const Twine &Anchor = "") {
+  OS << std::string(Num, '#') << " " << genEscaped(Text.str());
+  if (!Anchor.isTriviallyEmpty()) {
+    OS << " {#" << Anchor << "}";
   }
-
-static void writeNewLine(raw_ostream &OS) { OS << "\n\n"; }
-
-static void writeHeader(const Twine &Text, unsigned int Num, raw_ostream &OS) {
-  OS << std::string(Num, '#') + " " + Text << "\n\n";
+  OS << "\n";
 }
 
 static void writeFileDefinition(const ClangDocContext &CDCtx, const Location &L,
                                 raw_ostream &OS) {
-
   if (!CDCtx.RepositoryUrl) {
     OS << "*Defined at " << L.Filename << "#" << std::to_string(L.LineNumber)
        << "*";
   } else {
     OS << "*Defined at [" << L.Filename << "#" << std::to_string(L.LineNumber)
        << "](" << StringRef{*CDCtx.RepositoryUrl}
        << llvm::sys::path::relative_path(L.Filename) << "#"
        << std::to_string(L.LineNumber) << ")"
        << "*";
   }
-  OS << "\n\n";
+  OS << "\n";
 }
 
 static void writeDescription(const CommentInfo &I, raw_ostream &OS) {
   if (I.Kind == "FullComment") {
     for (const auto &Child : I.Children)
       writeDescription(*Child, OS);
   } else if (I.Kind == "ParagraphComment") {
     for (const auto &Child : I.Children)
       writeDescription(*Child, OS);
-    writeNewLine(OS);
+    OS << "\n";
   } else if (I.Kind == "BlockCommandComment") {
+    // TODO: @return is a block command and should be included in the "Returns" table.
+    // TODO: @see block commands should be grouped and rendered as "See Also" section.
+    // TODO: Figure out handling for @brief.
+    // TODO: What other block commands need special handling?
     OS << genEmphasis(I.Name);
     for (const auto &Child : I.Children)
       writeDescription(*Child, OS);
   } else if (I.Kind == "InlineCommandComment") {
-    OS << genEmphasis(I.Name) << " " << I.Text;
+    OS << genEmphasis(I.ParamName) << " " << I.Text;
+    for (const auto &Child : I.Children)
+      writeDescription(*Child, OS);
   } else if (I.Kind == "ParamCommandComment") {
+    // TODO: @param commands should included in the "Parameters" table.
     std::string Direction = I.Explicit ? (" " + I.Direction).str() : "";
     OS << genEmphasis(I.ParamName) << I.Text << Direction << "\n\n";
   } else if (I.Kind == "TParamCommandComment") {
+    // TODO: @tparam commands should included in the "Template Parameters" table.
     std::string Direction = I.Explicit ? (" " + I.Direction).str() : "";
     OS << genEmphasis(I.ParamName) << I.Text << Direction << "\n\n";
   } else if (I.Kind == "VerbatimBlockComment") {
+    // TODO: We should use ``` or indentation for verbatim blocks.
     for (const auto &Child : I.Children)
       writeDescription(*Child, OS);
   } else if (I.Kind == "VerbatimBlockLineComment") {
-    OS << I.Text;
-    writeNewLine(OS);
+    // TODO: We should use ` for verbatim block lines.
+    OS << I.Text << "\n";
   } else if (I.Kind == "VerbatimLineComment") {
-    OS << I.Text;
-    writeNewLine(OS);
+    // TODO: We should use ` for verbatim lines.
+    OS << I.Text << "\n";
   } else if (I.Kind == "HTMLStartTagComment") {
     if (I.AttrKeys.size() != I.AttrValues.size())
       return;
     std::string Buffer;
     llvm::raw_string_ostream Attrs(Buffer);
     for (unsigned Idx = 0; Idx < I.AttrKeys.size(); ++Idx)
       Attrs << " \"" << I.AttrKeys[Idx] << "=" << I.AttrValues[Idx] << "\"";
 
     std::string CloseTag = I.SelfClosing ? "/>" : ">";
-    writeLine("<" + I.Name + Attrs.str() + CloseTag, OS);
+    OS << "<" << I.Name << Attrs.str() << CloseTag << "\n";
   } else if (I.Kind == "HTMLEndTagComment") {
-    writeLine("</" + I.Name + ">", OS);
+    OS << "</" << I.Name << ">" << "\n";
   } else if (I.Kind == "TextComment") {
-    OS << I.Text;
+    OS << I.Text.substr(1) << "\n";
   } else {
     OS << "Unknown comment kind: " << I.Kind << ".\n\n";
   }
 }
 
 static void writeNameLink(const StringRef &CurrentPath, const Reference &R,
                           llvm::raw_ostream &OS) {
   llvm::SmallString<64> Path = R.getRelativeFilePath(CurrentPath);
   // Paths in Markdown use POSIX separators.
   llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
   llvm::sys::path::append(Path, llvm::sys::path::Style::posix,
                           R.getFileBaseName() + ".md");
   OS << "[" << R.Name << "](" << Path << ")";
 }
 
 static void genMarkdown(const ClangDocContext &CDCtx, const EnumInfo &I,
                         llvm::raw_ostream &OS) {
   if (I.Scoped)
-    writeLine("| enum class " + I.Name + " |", OS);
+    OS << "| enum class " << I.Name << " |" << "\n";
   else
-    writeLine("| enum " + I.Name + " |", OS);
-  writeLine("--", OS);
+    OS << "| enum " << I.Name << " |" << "\n";
+  OS << "--" << "\n";
 
   std::string Buffer;
   llvm::raw_string_ostream Members(Buffer);
   if (!I.Members.empty())
     for (const auto &N : I.Members)
       Members << "| " << N << " |\n";
-  writeLine(Members.str(), OS);
+  OS << Members.str() << "\n";
   if (I.DefLoc)
     writeFileDefinition(CDCtx, *I.DefLoc, OS);
 
   for (const auto &C : I.Description)
     writeDescription(C, OS);
 }
 
+static void genMarkdown(const ClangDocContext &CDCtx, const MemberTypeInfo &I,
+                        llvm::raw_ostream &OS) {
+  writeHeader(I.Name, 3, OS, I.Name);
+  OS << "\n";
+  std::string Access = getAccessSpelling(I.Access).str();
+  if (Access != "")
+    OS << genCode(Access + " " + I.Type.Name + " " + I.Name) << "\n";
+  else
+    OS << genCode(I.Type.Name + " " + I.Name) << "\n";
+  OS << "\n";
+}
+
 static void genMarkdown(const ClangDocContext &CDCtx, const FunctionInfo &I,
                         llvm::raw_ostream &OS) {
   std::string Buffer;
   llvm::raw_string_ostream Stream(Buffer);
   bool First = true;
   for (const auto &N : I.Params) {
     if (!First)
       Stream << ", ";
     Stream << N.Type.Name + " " + N.Name;
     First = false;
   }
   writeHeader(I.Name, 3, OS);
+  OS << "\n";
   std::string Access = getAccessSpelling(I.Access).str();
-  if (Access != "")
-    writeLine(genItalic(Access + " " + I.ReturnType.Type.Name + " " + I.Name +
-                        "(" + Stream.str() + ")"),
-              OS);
-  else
-    writeLine(genItalic(I.ReturnType.Type.Name + " " + I.Name + "(" +
-                        Stream.str() + ")"),
-              OS);
+  if (Access != "") {
+    OS << "```cpp\n";
+    OS << Access + " " + I.ReturnType.Type.Name + " " + I.Name + "(" + Stream.str() + ")\n";
+    OS << "```\n";
+  } else {
+    OS << "```cpp\n";
+    OS << I.ReturnType.Type.Name + " " + I.Name + "(" + Stream.str() + ")\n";
+    OS << "```\n";
+  }
   if (I.DefLoc)
     writeFileDefinition(CDCtx, *I.DefLoc, OS);
+  OS << "\n";
 
   for (const auto &C : I.Description)
     writeDescription(C, OS);
+
+  // Write function parameters as a table.
+
+  OS << "| Parameters | |\n";
+  OS << "| --- | --- |\n";
+  for (const auto &N : I.Params) {
+    OS << "| " << genCode(N.Name) << " | " << genCode(N.Type.Name) << "|\n";
+  }
+  OS << "\n";
+
+  // Write function return value as a table.
+
+  OS << "| Returns | |\n";
+  OS << "| --- | --- |\n";
+  OS << "| " << genCode(I.ReturnType.Type.Name) << " | " << "" << "|\n";
+  OS << "\n";
 }
 
 static void genMarkdown(const ClangDocContext &CDCtx, const NamespaceInfo &I,
                         llvm::raw_ostream &OS) {
   if (I.Name == "")
     writeHeader("Global Namespace", 1, OS);
   else
     writeHeader("namespace " + I.Name, 1, OS);
-  writeNewLine(OS);
+  OS << "\n";
 
   if (!I.Description.empty()) {
     for (const auto &C : I.Description)
       writeDescription(C, OS);
-    writeNewLine(OS);
+    OS << "\n";
   }
 
   llvm::SmallString<64> BasePath = I.getRelativeFilePath("");
 
   if (!I.ChildNamespaces.empty()) {
     writeHeader("Namespaces", 2, OS);
     for (const auto &R : I.ChildNamespaces) {
       OS << "* ";
       writeNameLink(BasePath, R, OS);
       OS << "\n";
     }
-    writeNewLine(OS);
+    OS << "\n";
   }
 
   if (!I.ChildRecords.empty()) {
     writeHeader("Records", 2, OS);
     for (const auto &R : I.ChildRecords) {
       OS << "* ";
       writeNameLink(BasePath, R, OS);
       OS << "\n";
     }
-    writeNewLine(OS);
+    OS << "\n";
   }
 
   if (!I.ChildFunctions.empty()) {
     writeHeader("Functions", 2, OS);
     for (const auto &F : I.ChildFunctions)
       genMarkdown(CDCtx, F, OS);
-    writeNewLine(OS);
+    OS << "\n";
   }
   if (!I.ChildEnums.empty()) {
     writeHeader("Enums", 2, OS);
     for (const auto &E : I.ChildEnums)
       genMarkdown(CDCtx, E, OS);
-    writeNewLine(OS);
+    OS << "\n";
   }
 }
 
 static void genMarkdown(const ClangDocContext &CDCtx, const RecordInfo &I,
                         llvm::raw_ostream &OS) {
-  writeHeader(getTagType(I.TagType) + " " + I.Name, 1, OS);
+  writeHeader(getTagType(I.TagType) + " " + I.Name, 1, OS, I.Name);
   if (I.DefLoc)
     writeFileDefinition(CDCtx, *I.DefLoc, OS);
+  OS << "\n";
 
   if (!I.Description.empty()) {
     for (const auto &C : I.Description)
       writeDescription(C, OS);
-    writeNewLine(OS);
+    OS << "\n";
   }
 
   std::string Parents = genReferenceList(I.Parents);
   std::string VParents = genReferenceList(I.VirtualParents);
   if (!Parents.empty() || !VParents.empty()) {
     if (Parents.empty())
-      writeLine("Inherits from " + VParents, OS);
+      OS << "Inherits from " << VParents << "\n";
     else if (VParents.empty())
-      writeLine("Inherits from " + Parents, OS);
+      OS << "Inherits from " << Parents << "\n";
     else
-      writeLine("Inherits from " + Parents + ", " + VParents, OS);
-    writeNewLine(OS);
+      OS << "Inherits from " << Parents << ", " << VParents << "\n";
+    OS << "\n";
   }
 
   if (!I.Members.empty()) {
     writeHeader("Members", 2, OS);
-    for (const auto &Member : I.Members) {
-      std::string Access = getAccessSpelling(Member.Access).str();
-      if (Access != "")
-        writeLine(Access + " " + Member.Type.Name + " " + Member.Name, OS);
-      else
-        writeLine(Member.Type.Name + " " + Member.Name, OS);
-    }
-    writeNewLine(OS);
+    OS << "\n";
+    for (const auto &Member : I.Members)
+      genMarkdown(CDCtx, Member, OS);
+    OS << "\n";
   }
 
   if (!I.ChildRecords.empty()) {
     writeHeader("Records", 2, OS);
     for (const auto &R : I.ChildRecords)
-      writeLine(R.Name, OS);
-    writeNewLine(OS);
+      OS << R.Name << "\n";
+    OS << "\n";
   }
   if (!I.ChildFunctions.empty()) {
     writeHeader("Functions", 2, OS);
     for (const auto &F : I.ChildFunctions)
       genMarkdown(CDCtx, F, OS);
-    writeNewLine(OS);
+    OS << "\n";
   }
   if (!I.ChildEnums.empty()) {
     writeHeader("Enums", 2, OS);
     for (const auto &E : I.ChildEnums)
       genMarkdown(CDCtx, E, OS);
-    writeNewLine(OS);
+    OS << "\n";
   }
 }
 
 static void serializeReference(llvm::raw_fd_ostream &OS, Index &I, int Level) {
   // Write out the heading level starting at ##
   OS << "##" << std::string(Level, '#') << " ";
   writeNameLink("", I, OS);
   OS << "\n";
if (!C.Children.empty()) {
       OS << "* " << Type << ": [" << C.Name << "](";
       if (!C.Path.empty())
         OS << C.Path << "/";
       OS << C.Name << ")\n";
     }
   }
   return llvm::Error::success();
 }
+
+static llvm::Error genTableOfContents(ClangDocContext &CDCtx) {
+  std::error_code FileErr;
+  llvm::SmallString<128> FilePath;
+  llvm::sys::path::native(CDCtx.OutDirectory, FilePath);
+  llvm::sys::path::append(FilePath, "_toc.yaml");
+  llvm::raw_fd_ostream OS(FilePath, FileErr, llvm::sys::fs::OF_None);
+  if (FileErr)
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "error creating toc file: " + FileErr.message());
+
+  // Table of Contents has the following structure:
+  //
+  //     toc:
+  //     - title: namespace foo
+  //       section:
+  //       - title: Classes
+  //         section:
+  //         - title: Foo
+  //           path: /path/to/foo.md
+
+  CDCtx.Idx.sort();
+  OS << "toc:\n";
+  for (auto C : CDCtx.Idx.Children) {
+    OS << "- title: " << C.Name << "\n";
+    llvm::SmallString<64> Path = C.getRelativeFilePath(""); // TODO: is this correct?
+    // Paths in Markdown use POSIX separators.
+    llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+    llvm::sys::path::append(Path, llvm::sys::path::Style::posix,
+                            C.getFileBaseName() + ".md");
+    OS << "  path: " << Path << "\n";
+
+    // TODO: Handle the nested elements.
+  }
+  return llvm::Error::success();
+}
+
 /// Generator for Markdown documentation.
 class MDGenerator : public Generator {
 public:
   static const char *Format;
 
   llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS,
                                  const ClangDocContext &CDCtx) override;
   llvm::Error createResources(ClangDocContext &CDCtx) override;
 };
 
 const char *MDGenerator::Format = "md";
 
 llvm::Error MDGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS,
                                             const ClangDocContext &CDCtx) {
+  OS << "[TOC]\n\n";
   switch (I->IT) {
   case InfoType::IT_namespace:
     genMarkdown(CDCtx, *static_cast<clang::doc::NamespaceInfo *>(I), OS);
     break;
   case InfoType::IT_record:
     genMarkdown(CDCtx, *static_cast<clang::doc::RecordInfo *>(I), OS);
     break;
   case InfoType::IT_enum:
llvm::Error MDGenerator::createResources(ClangDocContext &CDCtx) {
   if (Err)
     return Err;
 
   // Generate the index page.
   Err = genIndex(CDCtx);
   if (Err)
     return Err;
 
+  Err = genTableOfContents(CDCtx);
+  if (Err)
+    return Err;
+
   return llvm::Error::success();
 }
 
 static GeneratorRegistry::Add<MDGenerator> MD(MDGenerator::Format,
                                               "Generator for MD output.");
 
 // This anchor is used to force the linker to link in the generated object
 // file and thus register the generator.
 volatile int MDGeneratorAnchorSource = 0;
 
 } // namespace doc
 } // namespace clang