-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang-doc] fix CSS, JS paths for HTML Mustache generation #160360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-clang-tools-extra Author: Erick Velez (evelez7) ChangesResource creation not using the correct path after the patch that placed Full diff: https://github.com/llvm/llvm-project/pull/160360.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
index 1ab40aacbfe09..b37dc272ea156 100644
--- a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
@@ -274,11 +274,12 @@ Error MustacheHTMLGenerator::generateDocForInfo(Info *I, raw_ostream &OS,
}
Error MustacheHTMLGenerator::createResources(ClangDocContext &CDCtx) {
+ std::string ResourcePath(CDCtx.OutDirectory + "/html");
for (const auto &FilePath : CDCtx.UserStylesheets)
- if (Error Err = copyFile(FilePath, CDCtx.OutDirectory))
+ if (Error Err = copyFile(FilePath, ResourcePath))
return Err;
for (const auto &FilePath : CDCtx.JsScripts)
- if (Error Err = copyFile(FilePath, CDCtx.OutDirectory))
+ if (Error Err = copyFile(FilePath, ResourcePath))
return Err;
return Error::success();
}
diff --git a/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp
index 602058f5d9eb8..66575d85380f8 100644
--- a/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp
+++ b/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp
@@ -75,14 +75,14 @@ TEST(HTMLMustacheGeneratorTest, createResources) {
<< "Failed to create resources with valid UserStylesheets and JsScripts";
{
SmallString<256> PathBuf;
- llvm::sys::path::append(PathBuf, RootTestDirectory.path(),
+ llvm::sys::path::append(PathBuf, RootTestDirectory.path(), "html",
"clang-doc-mustache.css");
verifyFileContents(PathBuf, "CSS");
}
{
SmallString<256> PathBuf;
- llvm::sys::path::append(PathBuf, RootTestDirectory.path(), "mustache.js");
+ llvm::sys::path::append(PathBuf, RootTestDirectory.path(), "html", "mustache.js");
verifyFileContents(PathBuf, "JavaScript");
}
}
|
e6c68b9 to
4e6e429
Compare
|
This fix has to delete the HTML Mustache unit tests because I don't think it's possible to create nested temporary directories like what the unit test was doing. We'd need a temp I could avoid deleting the unit test if the resource files were placed one level above the Here's a link for the basic test output from this patch and here's one for current main which shows the broken CSS. |
|
Why do you have to delete the unit test? the basic error paths can still be checked there, right? even if you can't plumb in the right paths for template stuff, I'd think the basics of startup and when the generator error out would be testable. Next, its possible to control the behavior w/ options or by making the interfaces more easily testable by putting some interfaces into a header that the tests can include. |
|
for now, I'm OK w/ dropping the template buisness, and keeping the basic error checking, since its very cheap. |
4e6e429 to
6e0207f
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/198/builds/8144 Here is the relevant piece of the build log for the reference |

Resource creation not using the correct path after the patch that placed
HTML files into their own directory.