Skip to content

reapply [clang-doc] Add --asset option to clang-doc #96358

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 27, 2024
Merged

reapply [clang-doc] Add --asset option to clang-doc #96358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bb407e7
Revert "Revert "[clang-doc] Add --asset option to clang-doc" (#96354)"
PeterChou1 Jun 21, 2024
c7490ab
[clang-doc] address pr comments
PeterChou1 Jun 24, 2024
48b3dbf
[clang-doc] switch loading
PeterChou1 Jun 24, 2024
6321616
[clang-doc] remove extra --output
PeterChou1 Jun 25, 2024
900869e
[clang-doc] fix conflict
PeterChou1 Jun 25, 2024
ada6489
[clang-doc] fix default asset path
PeterChou1 Jun 25, 2024
2e6ee4c
Merge branch 'main' into re-revert-add-asset
PeterChou1 Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions clang-tools-extra/clang-doc/HTMLGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,18 @@ genStylesheetsHTML(StringRef InfoPath, const ClangDocContext &CDCtx) {
static std::vector<std::unique_ptr<TagNode>>
genJsScriptsHTML(StringRef InfoPath, const ClangDocContext &CDCtx) {
std::vector<std::unique_ptr<TagNode>> Out;

// index_json.js is part of every generated HTML file
SmallString<128> IndexJSONPath = computeRelativePath("", InfoPath);
auto IndexJSONNode = std::make_unique<TagNode>(HTMLTag::TAG_SCRIPT);
llvm::sys::path::append(IndexJSONPath, "index_json.js");
llvm::sys::path::native(IndexJSONPath, llvm::sys::path::Style::posix);
IndexJSONNode->Attributes.emplace_back("src", std::string(IndexJSONPath));
Out.emplace_back(std::move(IndexJSONNode));

for (const auto &FilePath : CDCtx.JsScripts) {
auto ScriptNode = std::make_unique<TagNode>(HTMLTag::TAG_SCRIPT);
SmallString<128> ScriptPath = computeRelativePath("", InfoPath);
auto ScriptNode = std::make_unique<TagNode>(HTMLTag::TAG_SCRIPT);
llvm::sys::path::append(ScriptPath, llvm::sys::path::filename(FilePath));
// Paths in HTML must be in posix-style
llvm::sys::path::native(ScriptPath, llvm::sys::path::Style::posix);
Expand Down Expand Up @@ -1069,7 +1078,7 @@ llvm::Error HTMLGenerator::createResources(ClangDocContext &CDCtx) {
if (Err)
return Err;
}
for (const auto &FilePath : CDCtx.FilesToCopy) {
for (const auto &FilePath : CDCtx.JsScripts) {
Err = CopyFile(FilePath, CDCtx.OutDirectory);
if (Err)
return Err;
Expand Down
6 changes: 2 additions & 4 deletions clang-tools-extra/clang-doc/Representation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,9 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx,
StringRef ProjectName, bool PublicOnly,
StringRef OutDirectory, StringRef SourceRoot,
StringRef RepositoryUrl,
std::vector<std::string> UserStylesheets,
std::vector<std::string> JsScripts)
std::vector<std::string> UserStylesheets)
: ECtx(ECtx), ProjectName(ProjectName), PublicOnly(PublicOnly),
OutDirectory(OutDirectory), UserStylesheets(UserStylesheets),
JsScripts(JsScripts) {
OutDirectory(OutDirectory), UserStylesheets(UserStylesheets) {
llvm::SmallString<128> SourceRootDir(SourceRoot);
if (SourceRoot.empty())
// If no SourceRoot was provided the current path is used as the default
Expand Down
5 changes: 1 addition & 4 deletions clang-tools-extra/clang-doc/Representation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,7 @@ struct ClangDocContext {
ClangDocContext(tooling::ExecutionContext *ECtx, StringRef ProjectName,
bool PublicOnly, StringRef OutDirectory, StringRef SourceRoot,
StringRef RepositoryUrl,
std::vector<std::string> UserStylesheets,
std::vector<std::string> JsScripts);
std::vector<std::string> UserStylesheets);
tooling::ExecutionContext *ECtx;
std::string ProjectName; // Name of project clang-doc is documenting.
bool PublicOnly; // Indicates if only public declarations are documented.
Expand All @@ -498,8 +497,6 @@ struct ClangDocContext {
std::vector<std::string> UserStylesheets;
// JavaScript files that will be imported in allHTML file.
std::vector<std::string> JsScripts;
// Other files that should be copied to OutDirectory, besides UserStylesheets.
std::vector<std::string> FilesToCopy;
Index Idx;
};

Expand Down
103 changes: 82 additions & 21 deletions clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ static llvm::cl::list<std::string> UserStylesheets(
llvm::cl::desc("CSS stylesheets to extend the default styles."),
llvm::cl::cat(ClangDocCategory));

static llvm::cl::opt<std::string> UserAssetPath(
"asset",
llvm::cl::desc("User supplied asset path to "
"override the default css and js files for html output"),
llvm::cl::cat(ClangDocCategory));

static llvm::cl::opt<std::string> SourceRoot("source-root", llvm::cl::desc(R"(
Directory where processed files are stored.
Links to definition locations will only be
Expand Down Expand Up @@ -127,16 +133,84 @@ std::string getFormatString() {
// GetMainExecutable (since some platforms don't support taking the
// address of main, and some platforms can't implement GetMainExecutable
// without being given the address of a function in the main executable).
std::string GetExecutablePath(const char *Argv0, void *MainAddr) {
std::string getExecutablePath(const char *Argv0, void *MainAddr) {
return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
}

llvm::Error getAssetFiles(clang::doc::ClangDocContext &CDCtx) {
using DirIt = llvm::sys::fs::directory_iterator;
std::error_code FileErr;
llvm::SmallString<128> FilePath(UserAssetPath);
for (DirIt DirStart = DirIt(UserAssetPath, FileErr),
DirEnd;
!FileErr && DirStart != DirEnd; DirStart.increment(FileErr)) {
FilePath = DirStart->path();
if (llvm::sys::fs::is_regular_file(FilePath)) {
if (llvm::sys::path::extension(FilePath) == ".css")
CDCtx.UserStylesheets.insert(CDCtx.UserStylesheets.begin(),
std::string(FilePath));
else if (llvm::sys::path::extension(FilePath) == ".js")
CDCtx.JsScripts.emplace_back(FilePath.str());
}
}
if (FileErr)
return llvm::createFileError(FilePath, FileErr);
return llvm::Error::success();
}

llvm::Error getDefaultAssetFiles(const char *Argv0,
clang::doc::ClangDocContext &CDCtx) {
void *MainAddr = (void *)(intptr_t)getExecutablePath;
std::string ClangDocPath = getExecutablePath(Argv0, MainAddr);
llvm::SmallString<128> NativeClangDocPath;
llvm::sys::path::native(ClangDocPath, NativeClangDocPath);

llvm::SmallString<128> AssetsPath;
AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
llvm::sys::path::append(AssetsPath, "..", "share", "clang-doc");
llvm::SmallString<128> DefaultStylesheet;
llvm::sys::path::native(AssetsPath, DefaultStylesheet);
llvm::sys::path::append(DefaultStylesheet,
"clang-doc-default-stylesheet.css");
llvm::SmallString<128> IndexJS;
llvm::sys::path::native(AssetsPath, IndexJS);
llvm::sys::path::append(IndexJS, "index.js");

if (!llvm::sys::fs::is_regular_file(IndexJS))
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"default index.js file missing at " +
IndexJS + "\n");

if (!llvm::sys::fs::is_regular_file(DefaultStylesheet))
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
"default clang-doc-default-stylesheet.css file missing at " +
DefaultStylesheet + "\n");

CDCtx.UserStylesheets.insert(CDCtx.UserStylesheets.begin(),
std::string(DefaultStylesheet));
CDCtx.JsScripts.emplace_back(IndexJS.str());

return llvm::Error::success();
}

llvm::Error getHtmlAssetFiles(const char *Argv0,
clang::doc::ClangDocContext &CDCtx) {
if (!UserAssetPath.empty() &&
!llvm::sys::fs::is_directory(std::string(UserAssetPath)))
llvm::outs() << "Asset path supply is not a directory: " << UserAssetPath
<< " falling back to default\n";
if (llvm::sys::fs::is_directory(std::string(UserAssetPath)))
return getAssetFiles(CDCtx);
return getDefaultAssetFiles(Argv0, CDCtx);
}

int main(int argc, const char **argv) {
llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
std::error_code OK;

const char *Overview =
R"(Generates documentation from source code and comments.
R"(Generates documentation from source code and comments.

Example usage for files without flags (default):

Expand Down Expand Up @@ -178,27 +252,14 @@ Example usage for a project using a compile commands database:
OutDirectory,
SourceRoot,
RepositoryUrl,
{UserStylesheets.begin(), UserStylesheets.end()},
{"index.js", "index_json.js"}};
{UserStylesheets.begin(), UserStylesheets.end()}
};

if (Format == "html") {
void *MainAddr = (void *)(intptr_t)GetExecutablePath;
std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
llvm::SmallString<128> NativeClangDocPath;
llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
llvm::SmallString<128> AssetsPath;
AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
llvm::sys::path::append(AssetsPath, "..", "share", "clang-doc");
llvm::SmallString<128> DefaultStylesheet;
llvm::sys::path::native(AssetsPath, DefaultStylesheet);
llvm::sys::path::append(DefaultStylesheet,
"clang-doc-default-stylesheet.css");
llvm::SmallString<128> IndexJS;
llvm::sys::path::native(AssetsPath, IndexJS);
llvm::sys::path::append(IndexJS, "index.js");
CDCtx.UserStylesheets.insert(CDCtx.UserStylesheets.begin(),
std::string(DefaultStylesheet));
CDCtx.FilesToCopy.emplace_back(IndexJS.str());
if (auto Err = getHtmlAssetFiles(argv[0], CDCtx)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After I've read this in some detail, I'm confused by this patch.

As far as I can tell, getAssetFiles() looks for all .css and .js files in the passed-in directory and appends them. Line 182 still wants the main js file to be called index.js, even if --asset is used (…right?), but only getDefaultAssetFiles() checks for the existence of that file.

The way I would've expected this to work is that --asset just overrides the value of AssetsPath. Is there more discussion on why this doesn't implemented that way somewhere that I missed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is an oversight on my part the clang-doc still expect users to provide a file called index.js even if we use the --asset option I've modify the pr to make it so that clang-doc uses whatever js file the user supplies

llvm::errs() << toString(std::move(Err)) << "\n";
return 1;
}
}

// Mapping phase
Expand Down
3 changes: 3 additions & 0 deletions clang-tools-extra/test/clang-doc/Inputs/test-assets/test.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
padding: 0;
}
1 change: 1 addition & 0 deletions clang-tools-extra/test/clang-doc/Inputs/test-assets/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("Hello, world!");
22 changes: 22 additions & 0 deletions clang-tools-extra/test/clang-doc/assets.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: rm -rf %t && mkdir %t
// RUN: clang-doc --format=html --output=%t --asset=%S/Inputs/test-assets --executor=standalone %s
// RUN: FileCheck %s -input-file=%t/index.html -check-prefix=INDEX
// RUN: FileCheck %s -input-file=%t/test.css -check-prefix=CSS
// RUN: FileCheck %s -input-file=%t/test.js -check-prefix=JS

// INDEX: <!DOCTYPE html>
// INDEX-NEXT: <meta charset="utf-8"/>
// INDEX-NEXT: <title>Index</title>
// INDEX-NEXT: <link rel="stylesheet" href="test.css"/>
// INDEX-NEXT: <script src="index_json.js"></script>
// INDEX-NEXT: <script src="test.js"></script>
// INDEX-NEXT: <header id="project-title"></header>
// INDEX-NEXT: <main>
// INDEX-NEXT: <div id="sidebar-left" path="" class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left" style="flex: 0 100%;"></div>
// INDEX-NEXT: </main>

// CSS: body {
// CSS-NEXT: padding: 0;
// CSS-NEXT: }

// JS: console.log("Hello, world!");
10 changes: 5 additions & 5 deletions clang-tools-extra/test/clang-doc/basic-project.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
// HTML-SHAPE-NEXT: <meta charset="utf-8"/>
// HTML-SHAPE-NEXT: <title>class Shape</title>
// HTML-SHAPE-NEXT: <link rel="stylesheet" href="../clang-doc-default-stylesheet.css"/>
// HTML-SHAPE-NEXT: <script src="../index.js"></script>
// HTML-SHAPE-NEXT: <script src="../index_json.js"></script>
// HTML-SHAPE-NEXT: <script src="{{.*}}index_json.js"></script>
// HTML-SHAPE-NEXT: <script src="{{.*}}index.js"></script>
// HTML-SHAPE-NEXT: <header id="project-title"></header>
// HTML-SHAPE-NEXT: <main>
// HTML-SHAPE-NEXT: <div id="sidebar-left" path="GlobalNamespace" class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left"></div>
Expand Down Expand Up @@ -122,8 +122,8 @@
// HTML-CALC-NEXT: <meta charset="utf-8"/>
// HTML-CALC-NEXT: <title>class Calculator</title>
// HTML-CALC-NEXT: <link rel="stylesheet" href="{{.*}}clang-doc-default-stylesheet.css"/>
// HTML-CALC-NEXT: <script src="{{.*}}index.js"></script>
// HTML-CALC-NEXT: <script src="{{.*}}index_json.js"></script>
// HTML-CALC-NEXT: <script src="{{.*}}index.js"></script>
// HTML-CALC-NEXT: <header id="project-title"></header>
// HTML-CALC-NEXT: <main>
// HTML-CALC-NEXT: <div id="sidebar-left" path="GlobalNamespace" class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left"></div>
Expand Down Expand Up @@ -200,8 +200,8 @@
// HTML-RECTANGLE-NEXT: <meta charset="utf-8"/>
// HTML-RECTANGLE-NEXT: <title>class Rectangle</title>
// HTML-RECTANGLE-NEXT: <link rel="stylesheet" href="{{.*}}clang-doc-default-stylesheet.css"/>
// HTML-RECTANGLE-NEXT: <script src="{{.*}}index.js"></script>
// HTML-RECTANGLE-NEXT: <script src="{{.*}}index_json.js"></script>
// HTML-RECTANGLE-NEXT: <script src="{{.*}}index.js"></script>
// HTML-RECTANGLE-NEXT: <header id="project-title"></header>
// HTML-RECTANGLE-NEXT: <main>
// HTML-RECTANGLE-NEXT: <div id="sidebar-left" path="GlobalNamespace" class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left"></div>
Expand Down Expand Up @@ -281,8 +281,8 @@
// HTML-CIRCLE-NEXT: <meta charset="utf-8"/>
// HTML-CIRCLE-NEXT: <title>class Circle</title>
// HTML-CIRCLE-NEXT: <link rel="stylesheet" href="{{.*}}clang-doc-default-stylesheet.css"/>
// HTML-CIRCLE-NEXT: <script src="{{.*}}index.js"></script>
// HTML-CIRCLE-NEXT: <script src="{{.*}}index_json.js"></script>
// HTML-CIRCLE-NEXT: <script src="{{.*}}index.js"></script>
// HTML-CIRCLE-NEXT: <header id="project-title"></header>
// HTML-CIRCLE-NEXT: <main>
// HTML-CIRCLE-NEXT: <div id="sidebar-left" path="GlobalNamespace" class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left"></div>
Expand Down
7 changes: 6 additions & 1 deletion clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ ClangDocContext
getClangDocContext(std::vector<std::string> UserStylesheets = {},
StringRef RepositoryUrl = "") {
ClangDocContext CDCtx{
{}, "test-project", {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
{}, "test-project", {}, {}, {}, RepositoryUrl, UserStylesheets};
CDCtx.UserStylesheets.insert(
CDCtx.UserStylesheets.begin(),
"../share/clang/clang-doc-default-stylesheet.css");
Expand Down Expand Up @@ -66,6 +66,7 @@ TEST(HTMLGeneratorTest, emitNamespaceHTML) {
<title>namespace Namespace</title>
<link rel="stylesheet" href="../clang-doc-default-stylesheet.css"/>
<link rel="stylesheet" href="../user-provided-stylesheet.css"/>
<script src="../index_json.js"></script>
<script src="../index.js"></script>
<header id="project-title">test-project</header>
<main>
Expand Down Expand Up @@ -176,6 +177,7 @@ TEST(HTMLGeneratorTest, emitRecordHTML) {
<meta charset="utf-8"/>
<title>class r</title>
<link rel="stylesheet" href="../../../clang-doc-default-stylesheet.css"/>
<script src="../../../index_json.js"></script>
<script src="../../../index.js"></script>
<header id="project-title">test-project</header>
<main>
Expand Down Expand Up @@ -290,6 +292,7 @@ TEST(HTMLGeneratorTest, emitFunctionHTML) {
<meta charset="utf-8"/>
<title></title>
<link rel="stylesheet" href="clang-doc-default-stylesheet.css"/>
<script src="index_json.js"></script>
<script src="index.js"></script>
<header id="project-title">test-project</header>
<main>
Expand Down Expand Up @@ -337,6 +340,7 @@ TEST(HTMLGeneratorTest, emitEnumHTML) {
<meta charset="utf-8"/>
<title></title>
<link rel="stylesheet" href="clang-doc-default-stylesheet.css"/>
<script src="index_json.js"></script>
<script src="index.js"></script>
<header id="project-title">test-project</header>
<main>
Expand Down Expand Up @@ -422,6 +426,7 @@ TEST(HTMLGeneratorTest, emitCommentHTML) {
<meta charset="utf-8"/>
<title></title>
<link rel="stylesheet" href="clang-doc-default-stylesheet.css"/>
<script src="index_json.js"></script>
<script src="index.js"></script>
<header id="project-title">test-project</header>
<main>
Expand Down
Loading