[clang-doc][NFC] Lift Mustache template generation from HTML

[evelez7]

To prepare for more backends to use Mustache templates, this patch lifts
the Mustache utilities from HTMLMustacheGenerator.cpp to
Generators.h. A MustacheGenerator interface is created to share code for
template creation.

[evelez7]

• [clang-doc][NFC] Lift Mustache template generation from HTML #167815 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

Author: Erick Velez (evelez7)

Changes

To prepare for more backends to use Mustache templates, this patch lifts
the Mustache utilities from HTMLMustacheGenerator.cpp to
Generators.h. A MustacheGenerator interface is created to share code for
template creation.

---

Patch is 20.49 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167815.diff

3 Files Affected:

• (modified) clang-tools-extra/clang-doc/Generators.cpp (+131)
• (modified) clang-tools-extra/clang-doc/Generators.h (+83)
• (modified) clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp (+28-175)

diff --git a/clang-tools-extra/clang-doc/Generators.cpp b/clang-tools-extra/clang-doc/Generators.cpp
index a5f6f1c7ea732..035e6e340bca8 100644
--- a/clang-tools-extra/clang-doc/Generators.cpp
+++ b/clang-tools-extra/clang-doc/Generators.cpp
@@ -7,6 +7,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "Generators.h"
+#include "support/File.h"
+#include "llvm/Support/TimeProfiler.h"
 
 LLVM_INSTANTIATE_REGISTRY(clang::doc::GeneratorRegistry)
 
@@ -42,6 +44,135 @@ std::string getTagType(TagTypeKind AS) {
   llvm_unreachable("Unknown TagTypeKind");
 }
 
+Error createFileOpenError(StringRef FileName, std::error_code EC) {
+  return createFileError("cannot open file " + FileName, EC);
+}
+
+Error MustacheGenerator::setupTemplate(
+    std::unique_ptr<MustacheTemplateFile> &Template, StringRef TemplatePath,
+    std::vector<std::pair<StringRef, StringRef>> Partials) {
+  auto T = MustacheTemplateFile::createMustacheFile(TemplatePath);
+  if (Error Err = T.takeError())
+    return Err;
+  Template = std::move(T.get());
+  for (const auto &[Name, FileName] : Partials)
+    if (auto Err = Template->registerPartialFile(Name, FileName))
+      return Err;
+  return Error::success();
+}
+
+Error MustacheGenerator::generateDocumentation(
+    StringRef RootDir, StringMap<std::unique_ptr<doc::Info>> Infos,
+    const clang::doc::ClangDocContext &CDCtx, std::string DirName) {
+  {
+    llvm::TimeTraceScope TS("Setup Templates");
+    if (auto Err = setupTemplateFiles(CDCtx))
+      return Err;
+  }
+
+  {
+    llvm::TimeTraceScope TS("Generate JSON for Mustache");
+    if (auto JSONGenerator = findGeneratorByName("json")) {
+      if (Error Err = JSONGenerator.get()->generateDocs(
+              RootDir, std::move(Infos), CDCtx))
+        return Err;
+    } else
+      return JSONGenerator.takeError();
+  }
+
+  SmallString<128> JSONPath;
+  sys::path::native(RootDir.str() + "/json", JSONPath);
+
+  StringMap<json::Value> JSONFileMap;
+  {
+    llvm::TimeTraceScope TS("Iterate JSON files");
+    std::error_code EC;
+    sys::fs::recursive_directory_iterator JSONIter(JSONPath, EC);
+    std::vector<json::Value> JSONFiles;
+    JSONFiles.reserve(Infos.size());
+    if (EC)
+      return createStringError("Failed to create directory iterator.");
+
+    SmallString<128> DocsDirPath(RootDir.str() + '/' + DirName);
+    sys::path::native(DocsDirPath);
+    if (auto EC = sys::fs::create_directories(DocsDirPath))
+      return createFileError(DocsDirPath, EC);
+    while (JSONIter != sys::fs::recursive_directory_iterator()) {
+      // create the same directory structure in the docs format dir
+      if (JSONIter->type() == sys::fs::file_type::directory_file) {
+        SmallString<128> DocsClonedPath(JSONIter->path());
+        sys::path::replace_path_prefix(DocsClonedPath, JSONPath, DocsDirPath);
+        if (auto EC = sys::fs::create_directories(DocsClonedPath))
+          return createFileError(DocsClonedPath, EC);
+      }
+
+      if (EC)
+        return createFileError("Failed to iterate: " + JSONIter->path(), EC);
+
+      auto Path = StringRef(JSONIter->path());
+      if (!Path.ends_with(".json")) {
+        JSONIter.increment(EC);
+        continue;
+      }
+
+      auto File = MemoryBuffer::getFile(Path);
+      if (EC = File.getError(); EC)
+        // TODO: Buffer errors to report later, look into using Clang
+        // diagnostics.
+        llvm::errs() << "Failed to open file: " << Path << " " << EC.message()
+                     << '\n';
+
+      auto Parsed = json::parse((*File)->getBuffer());
+      if (!Parsed)
+        return Parsed.takeError();
+      auto ValidJSON = Parsed.get();
+
+      std::error_code FileErr;
+      SmallString<128> DocsFilePath(JSONIter->path());
+      sys::path::replace_path_prefix(DocsFilePath, JSONPath, DocsDirPath);
+      sys::path::replace_extension(DocsFilePath, DirName);
+      raw_fd_ostream InfoOS(DocsFilePath, FileErr, sys::fs::OF_None);
+      if (FileErr)
+        return createFileOpenError(Path, FileErr);
+
+      auto RelativeRootPath = getRelativePathToRoot(DocsFilePath, DocsDirPath);
+      auto InfoTypeStr =
+          getInfoTypeStr(Parsed->getAsObject(), sys::path::stem(DocsFilePath));
+      if (!InfoTypeStr)
+        return InfoTypeStr.takeError();
+      if (Error Err = generateDocForJSON(*Parsed, InfoOS, CDCtx,
+                                         InfoTypeStr.get(), RelativeRootPath))
+        return Err;
+      JSONIter.increment(EC);
+    }
+  }
+
+  return Error::success();
+}
+
+Expected<std::string> MustacheGenerator::getInfoTypeStr(Object *Info,
+                                                        StringRef Filename) {
+  auto StrValue = (*Info)["InfoType"];
+  if (StrValue.kind() != json::Value::Kind::String)
+    return createStringError("JSON file '%s' does not contain key: 'InfoType'.",
+                             Filename.str().c_str());
+  auto ObjTypeStr = StrValue.getAsString();
+  if (!ObjTypeStr.has_value())
+    return createStringError(
+        "JSON file '%s' does not contain 'InfoType' field as a string.",
+        Filename.str().c_str());
+  return ObjTypeStr.value().str();
+}
+
+SmallString<128>
+MustacheGenerator::getRelativePathToRoot(StringRef PathToFile,
+                                         StringRef DocsRootPath) {
+  SmallString<128> PathVec(PathToFile);
+  // Remove filename, or else the relative path will have an extra "../"
+  sys::path::remove_filename(PathVec);
+  return computeRelativePath(DocsRootPath, PathVec);
+}
+
 llvm::Error Generator::createResources(ClangDocContext &CDCtx) {
   return llvm::Error::success();
 }
diff --git a/clang-tools-extra/clang-doc/Generators.h b/clang-tools-extra/clang-doc/Generators.h
index 92d3006e6002d..350db63aca368 100644
--- a/clang-tools-extra/clang-doc/Generators.h
+++ b/clang-tools-extra/clang-doc/Generators.h
@@ -14,8 +14,14 @@
 
 #include "Representation.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Mustache.h"
 #include "llvm/Support/Registry.h"
 
+using namespace llvm;
+using namespace llvm::json;
+using namespace llvm::mustache;
+
 namespace clang {
 namespace doc {
 
@@ -52,6 +58,83 @@ findGeneratorByName(llvm::StringRef Format);
 
 std::string getTagType(TagTypeKind AS);
 
+Error createFileOpenError(StringRef FileName, std::error_code EC);
+
+class MustacheTemplateFile {
+  BumpPtrAllocator Allocator;
+  StringSaver Saver;
+  MustacheContext Ctx;
+  Template T;
+  std::unique_ptr<MemoryBuffer> Buffer;
+
+public:
+  static Expected<std::unique_ptr<MustacheTemplateFile>>
+  createMustacheFile(StringRef FileName) {
+    ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrError =
+        MemoryBuffer::getFile(FileName);
+    if (auto EC = BufferOrError.getError())
+      return createFileOpenError(FileName, EC);
+    return std::make_unique<MustacheTemplateFile>(
+        std::move(BufferOrError.get()));
+  }
+
+  Error registerPartialFile(StringRef Name, StringRef FileName) {
+    ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrError =
+        MemoryBuffer::getFile(FileName);
+    if (auto EC = BufferOrError.getError())
+      return createFileOpenError(FileName, EC);
+
+    std::unique_ptr<MemoryBuffer> Buffer = std::move(BufferOrError.get());
+    StringRef FileContent = Buffer->getBuffer();
+    T.registerPartial(Name.str(), FileContent.str());
+    return Error::success();
+  }
+
+  void render(json::Value &V, raw_ostream &OS) { T.render(V, OS); }
+
+  MustacheTemplateFile(std::unique_ptr<MemoryBuffer> &&B)
+      : Saver(Allocator), Ctx(Allocator, Saver), T(B->getBuffer(), Ctx),
+        Buffer(std::move(B)) {}
+};
+
+struct MustacheGenerator {
+  Expected<std::string> getInfoTypeStr(Object *Info, StringRef Filename);
+
+  /// Used to find the relative path from the file to the format's docs root.
+  /// Mainly used for the HTML resource paths.
+  SmallString<128> getRelativePathToRoot(StringRef PathToFile,
+                                         StringRef DocsRootPath);
+  virtual ~MustacheGenerator() = default;
+
+  /// Initializes the template files from disk and calls setupTemplate to
+  /// register partials
+  virtual Error setupTemplateFiles(const ClangDocContext &CDCtx) = 0;
+
+  /// Populates templates with data from JSON and calls any specifics for the
+  /// format. For example, for HTML it will render the paths for CSS and JS.
+  virtual Error generateDocForJSON(json::Value &JSON, raw_fd_ostream &OS,
+                                   const ClangDocContext &CDCtx,
+                                   StringRef ObjectTypeStr,
+                                   StringRef RelativeRootPath) = 0;
+
+  /// Registers partials to templates.
+  Error setupTemplate(std::unique_ptr<MustacheTemplateFile> &Template,
+                      StringRef TemplatePath,
+                      std::vector<std::pair<StringRef, StringRef>> Partials);
+
+  /// \brief The main orchestrator for Mustache-based documentation.
+  ///
+  /// 1. Initializes templates files from disk by calling setupTemplateFiles.
+  /// 2. Calls the JSON generator to write JSON to disk.
+  /// 3. Iterates over the JSON files, recreates the directory structure from
+  /// JSON, and calls generateDocForJSON for each file.
+  /// 4. A file of the desired format is created.
+  Error generateDocumentation(StringRef RootDir,
+                              StringMap<std::unique_ptr<doc::Info>> Infos,
+                              const clang::doc::ClangDocContext &CDCtx,
+                              std::string DirName);
+};
+
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the generators.
 extern volatile int YAMLGeneratorAnchorSource;
diff --git a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
index 3650f66ec39bd..8402072a142aa 100644
--- a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
@@ -16,27 +16,16 @@
 #include "Representation.h"
 #include "support/File.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Mustache.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/TimeProfiler.h"
-
-using namespace llvm;
-using namespace llvm::json;
-using namespace llvm::mustache;
 
 namespace clang {
 namespace doc {
-static Error generateDocForJSON(json::Value &JSON, StringRef Filename,
-                                StringRef Path, raw_fd_ostream &OS,
-                                const ClangDocContext &CDCtx,
-                                StringRef HTMLRootPath);
 
-static Error createFileOpenError(StringRef FileName, std::error_code EC) {
-  return createFileError("cannot open file " + FileName, EC);
-}
+static std::unique_ptr<MustacheTemplateFile> NamespaceTemplate = nullptr;
+
+static std::unique_ptr<MustacheTemplateFile> RecordTemplate = nullptr;
 
-class MustacheHTMLGenerator : public Generator {
+class MustacheHTMLGenerator : public Generator, public MustacheGenerator {
 public:
   static const char *Format;
   Error generateDocs(StringRef RootDir,
@@ -45,64 +34,16 @@ class MustacheHTMLGenerator : public Generator {
   Error createResources(ClangDocContext &CDCtx) override;
   Error generateDocForInfo(Info *I, raw_ostream &OS,
                            const ClangDocContext &CDCtx) override;
+  Error setupTemplateFiles(const ClangDocContext &CDCtx) override;
+  Error generateDocForJSON(json::Value &JSON, raw_fd_ostream &OS,
+                           const ClangDocContext &CDCtx, StringRef ObjTypeStr,
+                           StringRef RelativeRootPath) override;
+  // Populates templates with CSS stylesheets, JS scripts paths.
+  Error setupTemplateResources(const ClangDocContext &CDCtx, json::Value &V,
+                               SmallString<128> RelativeRootPath);
 };
 
-class MustacheTemplateFile {
-  BumpPtrAllocator Allocator;
-  StringSaver Saver;
-  MustacheContext Ctx;
-  Template T;
-  std::unique_ptr<MemoryBuffer> Buffer;
-
-public:
-  static Expected<std::unique_ptr<MustacheTemplateFile>>
-  createMustacheFile(StringRef FileName) {
-    ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrError =
-        MemoryBuffer::getFile(FileName);
-    if (auto EC = BufferOrError.getError())
-      return createFileOpenError(FileName, EC);
-    return std::make_unique<MustacheTemplateFile>(
-        std::move(BufferOrError.get()));
-  }
-
-  Error registerPartialFile(StringRef Name, StringRef FileName) {
-    ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrError =
-        MemoryBuffer::getFile(FileName);
-    if (auto EC = BufferOrError.getError())
-      return createFileOpenError(FileName, EC);
-
-    std::unique_ptr<MemoryBuffer> Buffer = std::move(BufferOrError.get());
-    StringRef FileContent = Buffer->getBuffer();
-    T.registerPartial(Name.str(), FileContent.str());
-    return Error::success();
-  }
-
-  void render(json::Value &V, raw_ostream &OS) { T.render(V, OS); }
-
-  MustacheTemplateFile(std::unique_ptr<MemoryBuffer> &&B)
-      : Saver(Allocator), Ctx(Allocator, Saver), T(B->getBuffer(), Ctx),
-        Buffer(std::move(B)) {}
-};
-
-static std::unique_ptr<MustacheTemplateFile> NamespaceTemplate = nullptr;
-
-static std::unique_ptr<MustacheTemplateFile> RecordTemplate = nullptr;
-
-static Error
-setupTemplate(std::unique_ptr<MustacheTemplateFile> &Template,
-              StringRef TemplatePath,
-              std::vector<std::pair<StringRef, StringRef>> Partials) {
-  auto T = MustacheTemplateFile::createMustacheFile(TemplatePath);
-  if (Error Err = T.takeError())
-    return Err;
-  Template = std::move(T.get());
-  for (const auto &[Name, FileName] : Partials)
-    if (auto Err = Template->registerPartialFile(Name, FileName))
-      return Err;
-  return Error::success();
-}
-
-static Error setupTemplateFiles(const clang::doc::ClangDocContext &CDCtx) {
+Error MustacheHTMLGenerator::setupTemplateFiles(const ClangDocContext &CDCtx) {
   // Template files need to use the native path when they're opened,
   // but have to be used in POSIX style when used in HTML.
   auto ConvertToNative = [](std::string &&Path) -> std::string {
@@ -138,95 +79,20 @@ static Error setupTemplateFiles(const clang::doc::ClangDocContext &CDCtx) {
 Error MustacheHTMLGenerator::generateDocs(
     StringRef RootDir, StringMap<std::unique_ptr<doc::Info>> Infos,
     const clang::doc::ClangDocContext &CDCtx) {
-  {
-    llvm::TimeTraceScope TS("Setup Templates");
-    if (auto Err = setupTemplateFiles(CDCtx))
-      return Err;
-  }
-
-  {
-    llvm::TimeTraceScope TS("Generate JSON for Mustache");
-    if (auto JSONGenerator = findGeneratorByName("json")) {
-      if (Error Err = JSONGenerator.get()->generateDocs(
-              RootDir, std::move(Infos), CDCtx))
-        return Err;
-    } else
-      return JSONGenerator.takeError();
-  }
-  SmallString<128> JSONPath;
-  sys::path::native(RootDir.str() + "/json", JSONPath);
-
-  StringMap<json::Value> JSONFileMap;
-  {
-    llvm::TimeTraceScope TS("Iterate JSON files");
-    std::error_code EC;
-    sys::fs::recursive_directory_iterator JSONIter(JSONPath, EC);
-    std::vector<json::Value> JSONFiles;
-    JSONFiles.reserve(Infos.size());
-    if (EC)
-      return createStringError("Failed to create directory iterator.");
-
-    SmallString<128> HTMLDirPath(RootDir.str() + "/html");
-    if (auto EC = sys::fs::create_directories(HTMLDirPath))
-      return createFileError(HTMLDirPath, EC);
-    while (JSONIter != sys::fs::recursive_directory_iterator()) {
-      // create the same directory structure in the HTML dir
-      if (JSONIter->type() == sys::fs::file_type::directory_file) {
-        SmallString<128> HTMLClonedPath(JSONIter->path());
-        sys::path::replace_path_prefix(HTMLClonedPath, JSONPath, HTMLDirPath);
-        if (auto EC = sys::fs::create_directories(HTMLClonedPath))
-          return createFileError(HTMLClonedPath, EC);
-      }
-
-      if (EC)
-        return createFileError("Failed to iterate: " + JSONIter->path(), EC);
-
-      auto Path = StringRef(JSONIter->path());
-      if (!Path.ends_with(".json")) {
-        JSONIter.increment(EC);
-        continue;
-      }
-
-      auto File = MemoryBuffer::getFile(Path);
-      if (EC = File.getError(); EC)
-        // TODO: Buffer errors to report later, look into using Clang
-        // diagnostics.
-        llvm::errs() << "Failed to open file: " << Path << " " << EC.message()
-                     << '\n';
-
-      auto Parsed = json::parse((*File)->getBuffer());
-      if (!Parsed)
-        return Parsed.takeError();
-
-      std::error_code FileErr;
-      SmallString<128> HTMLFilePath(JSONIter->path());
-      sys::path::replace_path_prefix(HTMLFilePath, JSONPath, HTMLDirPath);
-      sys::path::replace_extension(HTMLFilePath, "html");
-      raw_fd_ostream InfoOS(HTMLFilePath, FileErr, sys::fs::OF_None);
-      if (FileErr)
-        return createFileOpenError(Path, FileErr);
-
-      if (Error Err =
-              generateDocForJSON(*Parsed, sys::path::stem(HTMLFilePath),
-                                 HTMLFilePath, InfoOS, CDCtx, HTMLDirPath))
-        return Err;
-      JSONIter.increment(EC);
-    }
-  }
-
-  return Error::success();
+  return generateDocumentation(RootDir, std::move(Infos), CDCtx, "html");
 }
 
-static Error setupTemplateValue(const ClangDocContext &CDCtx, json::Value &V,
-                                SmallString<128> RelativeHTMLPath) {
+Error MustacheHTMLGenerator::setupTemplateResources(
+    const ClangDocContext &CDCtx, json::Value &V,
+    SmallString<128> RelativeRootPath) {
   V.getAsObject()->insert({"ProjectName", CDCtx.ProjectName});
   json::Value StylesheetArr = Array();
-  sys::path::native(RelativeHTMLPath, sys::path::Style::posix);
+  sys::path::native(RelativeRootPath, sys::path::Style::posix);
 
   auto *SSA = StylesheetArr.getAsArray();
   SSA->reserve(CDCtx.UserStylesheets.size());
   for (const auto &FilePath : CDCtx.UserStylesheets) {
-    SmallString<128> StylesheetPath = RelativeHTMLPath;
+    SmallString<128> StylesheetPath = RelativeRootPath;
     sys::path::append(StylesheetPath, sys::path::Style::posix,
                       sys::path::filename(FilePath));
     SSA->emplace_back(StylesheetPath);
@@ -237,7 +103,7 @@ static Error setupTemplateValue(const ClangDocContext &CDCtx, json::Value &V,
   auto *SCA = ScriptArr.getAsArray();
   SCA->reserve(CDCtx.JsScripts.size());
   for (auto Script : CDCtx.JsScripts) {
-    SmallString<128> JsPath = RelativeHTMLPath;
+    SmallString<128> JsPath = RelativeRootPath;
     sys::path::append(JsPath, sys::path::Style::posix,
                       sys::path::filename(Script));
     SCA->emplace_back(JsPath);
@@ -246,31 +112,18 @@ static Error setupTemplateValue(const ClangDocContext &CDCtx, json::Value &V,
   return Error::success();
 }
 
-static Error generateDocForJSON(json::Value &JSON, StringRef Filename,
-                                StringRef Path, raw_fd_ostream &OS,
-                                const ClangDocContext &CDCtx,
-                                StringRef HTMLRootPath) {
-  auto StrValue = (*JSON.getAsObject())["InfoType"];
-  if (StrValue.kind() != json::Value::Kind::String)
-    return createStringError("JSON file '%s' does not contain key: 'InfoType'.",
-                             Filename.str().c_str());
-  auto ObjTypeStr = StrValue.getAsString();
-  if (!ObjTypeStr.has_value())
-    return createStringError(
-        "JSON file '%s' does not contain 'InfoType' field as a string.",
-        Filename.str().c_str());
-
-  SmallString<128> PathVec(Path);
-  // Remove filename, or else the relative path will have an extra "../"
-  sys::path::remove_filename(PathVec);
-  auto RelativeHTMLPath = computeRelativePath(HTMLRootPath, PathVec);
-  if (ObjTypeStr.value() == "namespace") {
-    if (auto Err = setupTemplateValue(CDCtx, JSON, RelativeHTMLPath))
+Error MustacheHTMLGenerator::generateDocForJSON(json::Value &JSON,
+                                                raw_fd_ostream &OS,
+                                                const ClangDocContext &CDCtx,
+                                                StringRef ObjTypeStr,
+                                                StringRef RelativeRootPath) {
+  if (ObjTypeStr == "namespace") {
+    if (auto Err = setupTemplateResources(CDCtx, JSON...
[truncated]

[evelez7]

Just to verify that HTML output is unaffected: https://erickvelez.com/clang-doc-mustache-output/pr167815/GlobalNamespace/

[ilovepi]

Mostly LGTM, but we need a couple things addressed first. The only major issue I see are the using statements. That will make this a bit more annoying, but hopefully clangd will make it not so bad. or maybe you can auto apply the fixes w/ clang-tidy?

The rest of the comments are just things I noticed when reading the code, mostly odd instances of the style guide bracing rules that made the code harder to read (for me).

You can also mark this change as an NFC, since you're just moving code around w/o adding any new logic. It's really a pure refactor.

nit: The start of the title should be capitalized Lift Mustache ...

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[petrhosek]

Having both generateDocumentation and generateDocs as methods in the same class is a bit confusing. Would it make sense for generateDocumentation to be virtual and generateDocs and MustacheHTMLGenerator to be the overridden version?

[evelez7]

Having both generateDocumentation and generateDocs as methods in the same class is a bit confusing.

Agreed.

Would it make sense for generateDocumentation to be virtual and generateDocs and MustacheHTMLGenerator to be the overridden version?

I'm not sure I fully understand. generateDocs is still used by the older backends, so their function signatures would have to be changed to comply with generateDocumentation as the base since it requires another std::string. I just realized that having the new MustacheGenerator interface inherit from Generator would be nice, but we arrive at the same problem. Maybe I'm misunderstanding?

What I can do pretty easily is make generateDocumentation protected (and change the name for less confusion) so that generateDocs is the public entry point. All it does in HTMLMustacheGenerator is call generateDocumentation. What do you think?

[ilovepi]

you could update the GenerateDocs() signature to have a trailing string argument w/ a default value, right? then there isn't a signature mismatch.

[evelez7]

I refactored the original generateDocs to be generateDocumentation with a default string arg to account for the DirName needed by MustacheGenerator. MustacheGenerator now inherits from Generator which seems better. Now there is just one shared entry point. Please let me know what you think.

[ilovepi]

why not just leave the name in place and update the mustache implementation to call this API?

[ilovepi]

I guess they're kind of equivalent, but the other way sounds easier.

[evelez7]

Yeah this ended up being a bit more invasive than I had planned. I didn't want to touch any of the older generators, but had to for the default arg and thought might as well use the nicer name. I can revert back to the old name if you want, it ends up being functionally the same thing.

[ilovepi]

I don’t think it matters much in the end. The diff would have been a bit less, I think, but it’s a rather minor difference imo.

[ilovepi]

If this is passing ci I think it’s fine to land.

[evelez7]

Merge activity

• Nov 19, 8:41 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Nov 19, 8:42 PM UTC: @evelez7 merged this pull request with Graphite.