[clang-doc] Add helpers for Template config #138062
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 1, 2025
This was referenced May 1, 2025
|
@llvm/pr-subscribers-clang-tools-extra Author: Paul Kirth (ilovepi) ChangesThis patch adds or fills in some helper functions related to template Co-authored-by: Peter Chou <[email protected]> Full diff: https://github.com/llvm/llvm-project/pull/138062.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
index 1288e4fbbc983..593d5d1221f44 100644
--- a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
@@ -61,7 +61,41 @@ 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 (auto EC = T.getError())
+ return createFileError("cannot open file", EC);
+ 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) {
+ std::string NamespaceFilePath =
+ CDCtx.MustacheTemplates.lookup("namespace-template");
+ std::string ClassFilePath = CDCtx.MustacheTemplates.lookup("class-template");
+ std::string CommentFilePath =
+ CDCtx.MustacheTemplates.lookup("comments-template");
+ std::string FunctionFilePath =
+ CDCtx.MustacheTemplates.lookup("function-template");
+ std::string EnumFilePath = CDCtx.MustacheTemplates.lookup("enum-template");
+ std::vector<std::pair<StringRef, StringRef>> Partials = {
+ {"Comments", CommentFilePath},
+ {"FunctionPartial", FunctionFilePath},
+ {"EnumPartial", EnumFilePath}};
+
+ if (Error Err = setupTemplate(NamespaceTemplate, NamespaceFilePath, Partials))
+ return Err;
+
+ if (Error Err = setupTemplate(RecordTemplate, ClassFilePath, Partials))
+ return Err;
+
return Error::success();
}
|
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
77bb241 to
516f2b2
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
57c7cf8 to
5b34441
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
5b34441 to
10fe47d
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
516f2b2 to
50755c0
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
10fe47d to
48e9893
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
50755c0 to
1ef8e3c
Compare
ilovepi
commented
May 7, 2025
clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp
Outdated
Show resolved
Hide resolved
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
48e9893 to
dd0bf73
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
1ef8e3c to
0ec149d
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
dd0bf73 to
b67b6ac
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
0ec149d to
f8205f7
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
b67b6ac to
ffafdb7
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
f8205f7 to
1079b7d
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
6d311cf to
fb0db52
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
1925647 to
de0222f
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
fb0db52 to
e918ff1
Compare
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-generators
branch
from
de0222f to
187b130
Compare
Base automatically changed from
users/ilovepi/clang-doc-mustache-generators
to
main
May 17, 2025 00:30
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
2 times, most recently
from
442fe2f to
3d87071
Compare
evelez7
approved these changes
May 17, 2025
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
3d87071 to
7f91d87
Compare
mysterymath
approved these changes
May 20, 2025
This patch adds or fills in some helper functions related to template setup when initializing the mustache backend. It was split from #133161. Co-authored-by: Peter Chou <[email protected]>
ilovepi
force-pushed
the
users/ilovepi/clang-doc-mustache-templete-helper
branch
from
7f91d87 to
86dc2be
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/9814 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/7205 Here is the relevant piece of the build log for the reference |
|
Reading files from unit tests is a bit of an anti-pattern: It requires putting the absolute path to the tests somewhere (you're doing this with a config file here), and it means you easily run tests on a different machine than the one you're building on. Could you restructure this test to not read a file? Or, if you need to run a file, can you do that from a lit test, where the path from build artifacts and test inputs is abstracted away enough that the build/test separation still works? |
nico
added a commit
that referenced
this pull request
May 23, 2025
Left a comment here: #138062 (comment)
I also don't really love the way this is shaped. We're trying to land functionality we want/need in clang-doc that a contributor more or less abandoned. Support to run the tool as a whole is further up the stack. I think it may be hard to reorder the patches to speed that up. As an alternative, we could disable the unit tests for the new feature, since its expected to be experimental for some time, and move the testing into lit tests. |
ilovepi
added a commit
that referenced
this pull request
May 23, 2025
In #138062 it was brought up that this was an anti-pattern. We'll need to Migrate all of the mustache unittests to lit tests, and disable them until tool support lands.
That sounds like a good way forward to me :) |
ilovepi
added a commit
that referenced
this pull request
May 23, 2025
In #138062 it was brought up that this was an anti-pattern. We'll need to Migrate all of the mustache unittests that need to read template files to lit tests, and disable them until tool support lands.
|
#141269 should fix the issue for now. I'll clean up the rest of the stack to avoid reintroducing this problem. |
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
This patch adds or fills in some helper functions related to template setup when initializing the mustache backend. It was split from llvm#133161. Co-authored-by: Peter Chou <[email protected]>
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
Left a comment here: llvm#138062 (comment)
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
In llvm#138062 it was brought up that this was an anti-pattern. We'll need to Migrate all of the mustache unittests that need to read template files to lit tests, and disable them until tool support lands.
ajaden-codes
pushed a commit
to Jaddyen/llvm-project
that referenced
this pull request
Jun 6, 2025
This patch adds or fills in some helper functions related to template setup when initializing the mustache backend. It was split from llvm#133161. Co-authored-by: Peter Chou <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds or fills in some helper functions related to template
setup when initializing the mustache backend. It was split from #133161.
Co-authored-by: Peter Chou [email protected]