Skip to content

Conversation

@ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Mar 21, 2025

Currently, when we set URLs from JS, we set them only using the protocol
and host locations. This works fine when docs are served from the base
directory of the site, but if you want to nest it under another
directory, our JS fails to set the correct path, leading to broken
links.

This patch adds a --base option to specify the path prefix to use, which
is set in the generated index_json.js file. index.json can then fill in
the prefix appropriately when generating links in a browser. This flag
has no effect for non HTML output.

Given an index hosted at: www.docs.com/base_directory/index.html
we used to generate the following link:
www.docs.com/file.html
Using --base base_directory we now generate:
www.docs.com/base_directory/file.html

This allows such links to work when hosting pages without using a custom
index.js.

Copy link
Contributor Author

ilovepi commented Mar 21, 2025

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

@ilovepi ilovepi requested review from PeterChou1 and petrhosek March 21, 2025 22:45
@ilovepi ilovepi marked this pull request as ready for review March 21, 2025 22:45
@llvmbot
Copy link
Member

llvmbot commented Mar 21, 2025

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

Author: Paul Kirth (ilovepi)

Changes

Currently, when we set URLs from JS, we set them only using the protocol
and host locations. This works fine when docs are served from the base
directory of the site, but if you want to nest it under another
directory, our JS fails to set the correct path, leading to broken
links.

This patch adds a --base option to specify the path prefix to use, which
is set in the generated index_json.js file. index.json can then fill in
the prefix appropriately when generating links in a browser. This flag
has no effect for non HTML output.

Given an index hosted at: www.docs.com/base_directory/index.html
we used to generate the following link:
www.docs.com/file.html
Using --base base_directory we now generate:
www.docs.com/base_directory/file.html

This allows such links to work when hosting pages without using a custom
index.js.


Full diff: https://github.com/llvm/llvm-project/pull/132482.diff

8 Files Affected:

  • (modified) clang-tools-extra/clang-doc/HTMLGenerator.cpp (+5)
  • (modified) clang-tools-extra/clang-doc/Representation.cpp (+2-2)
  • (modified) clang-tools-extra/clang-doc/Representation.h (+2)
  • (modified) clang-tools-extra/clang-doc/assets/index.js (+1-1)
  • (modified) clang-tools-extra/clang-doc/tool/ClangDocMain.cpp (+7)
  • (modified) clang-tools-extra/test/clang-doc/assets.cpp (+2-2)
  • (modified) clang-tools-extra/test/clang-doc/test-path-abs.cpp (+2-1)
  • (modified) clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp (+5-4)
diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
index a8404479569f9..9b95d082fdfe7 100644
--- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -1078,6 +1078,11 @@ static llvm::Error serializeIndex(ClangDocContext &CDCtx) {
   std::replace(RootPathEscaped.begin(), RootPathEscaped.end(), '\\', '/');
   OS << "var RootPath = \"" << RootPathEscaped << "\";\n";
 
+  llvm::SmallString<128> Base(CDCtx.Base);
+  std::string BaseEscaped = Base.str().str();
+  std::replace(BaseEscaped.begin(), BaseEscaped.end(), '\\', '/');
+  OS << "var Base = \"" << BaseEscaped << "\";\n";
+
   CDCtx.Idx.sort();
   llvm::json::OStream J(OS, 2);
   std::function<void(Index)> IndexToJSON = [&](const Index &I) {
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
index 4da93b24c131f..fd206fb6a18cc 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -367,10 +367,10 @@ void Index::sort() {
 ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx,
                                  StringRef ProjectName, bool PublicOnly,
                                  StringRef OutDirectory, StringRef SourceRoot,
-                                 StringRef RepositoryUrl,
+                                 StringRef RepositoryUrl, StringRef Base,
                                  std::vector<std::string> UserStylesheets)
     : ECtx(ECtx), ProjectName(ProjectName), PublicOnly(PublicOnly),
-      OutDirectory(OutDirectory), UserStylesheets(UserStylesheets) {
+      OutDirectory(OutDirectory), UserStylesheets(UserStylesheets), Base(Base) {
   llvm::SmallString<128> SourceRootDir(SourceRoot);
   if (SourceRoot.empty())
     // If no SourceRoot was provided the current path is used as the default
diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h
index bb0c534af7b74..562b8938b77ef 100644
--- a/clang-tools-extra/clang-doc/Representation.h
+++ b/clang-tools-extra/clang-doc/Representation.h
@@ -508,6 +508,7 @@ struct ClangDocContext {
   ClangDocContext(tooling::ExecutionContext *ECtx, StringRef ProjectName,
                   bool PublicOnly, StringRef OutDirectory, StringRef SourceRoot,
                   StringRef RepositoryUrl,
+                  StringRef Base,
                   std::vector<std::string> UserStylesheets);
   tooling::ExecutionContext *ECtx;
   std::string ProjectName; // Name of project clang-doc is documenting.
@@ -523,6 +524,7 @@ struct ClangDocContext {
   std::vector<std::string> UserStylesheets;
   // JavaScript files that will be imported in allHTML file.
   std::vector<std::string> JsScripts;
+  StringRef Base;
   Index Idx;
 };
 
diff --git a/clang-tools-extra/clang-doc/assets/index.js b/clang-tools-extra/clang-doc/assets/index.js
index 58a92f049f19f..ae3017dab93f4 100644
--- a/clang-tools-extra/clang-doc/assets/index.js
+++ b/clang-tools-extra/clang-doc/assets/index.js
@@ -3,7 +3,7 @@ function genLink(Ref) {
   // serving via a http server or viewing from a local
   var Path = window.location.protocol.startsWith("file") ?
       `${window.location.protocol}//${RootPath}/${Ref.Path}` :
-      `${window.location.protocol}//${window.location.host}/${Ref.Path}`;
+      `${window.location.protocol}//${window.location.host}/${Base}/${Ref.Path}`;
   if (Ref.RefType === "namespace") {
     Path = `${Path}/index.html`
   } else if (Ref.Path === "") {
diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
index 2ce707feb3d5e..fc5962b3b6e72 100644
--- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -67,6 +67,12 @@ static llvm::cl::opt<std::string>
                  llvm::cl::desc("Directory for outputting generated files."),
                  llvm::cl::init("docs"), llvm::cl::cat(ClangDocCategory));
 
+static llvm::cl::opt<std::string>
+    BaseDirectory("base",
+                 llvm::cl::desc("Base Directory for generated documentation."),
+                 llvm::cl::init(""), llvm::cl::cat(ClangDocCategory));
+
+
 static llvm::cl::opt<bool>
     PublicOnly("public", llvm::cl::desc("Document only public declarations."),
                llvm::cl::init(false), llvm::cl::cat(ClangDocCategory));
@@ -268,6 +274,7 @@ Example usage for a project using a compile commands database:
       OutDirectory,
       SourceRoot,
       RepositoryUrl,
+      BaseDirectory,
       {UserStylesheets.begin(), UserStylesheets.end()}
   };
 
diff --git a/clang-tools-extra/test/clang-doc/assets.cpp b/clang-tools-extra/test/clang-doc/assets.cpp
index d5a2d20e92240..c5933e504f6b9 100644
--- a/clang-tools-extra/test/clang-doc/assets.cpp
+++ b/clang-tools-extra/test/clang-doc/assets.cpp
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t && mkdir %t
-// RUN: clang-doc --format=html --output=%t --asset=%S/Inputs/test-assets --executor=standalone %s
+// RUN: clang-doc --format=html --output=%t --asset=%S/Inputs/test-assets --executor=standalone %s --base base_dir
 // 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
@@ -19,4 +19,4 @@
 // CSS-NEXT:     padding: 0;
 // CSS-NEXT: }
 
-// JS: console.log("Hello, world!");
\ No newline at end of file
+// JS: console.log("Hello, world!");
diff --git a/clang-tools-extra/test/clang-doc/test-path-abs.cpp b/clang-tools-extra/test/clang-doc/test-path-abs.cpp
index 292a2a3b5474d..8875a3a73ab7e 100644
--- a/clang-tools-extra/test/clang-doc/test-path-abs.cpp
+++ b/clang-tools-extra/test/clang-doc/test-path-abs.cpp
@@ -1,6 +1,7 @@
 // RUN: rm -rf %t && mkdir -p %t
-// RUN: clang-doc --format=html --executor=standalone %s --output=%t
+// RUN: clang-doc --format=html --executor=standalone %s --output=%t --base base_dir
 // RUN: FileCheck %s -input-file=%t/index_json.js  -check-prefix=JSON-INDEX
 
 // JSON-INDEX: var RootPath = "{{.*}}test-path-abs.cpp.tmp";
+// JSON-INDEX-NEXT: var Base = "base_dir";
 
diff --git a/clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
index 97afa12cab6d3..64363923ade8f 100644
--- a/clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ b/clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -28,9 +28,9 @@ std::unique_ptr<Generator> getHTMLGenerator() {
 
 ClangDocContext
 getClangDocContext(std::vector<std::string> UserStylesheets = {},
-                   StringRef RepositoryUrl = "") {
-  ClangDocContext CDCtx{
-      {}, "test-project", {}, {}, {}, RepositoryUrl, UserStylesheets};
+                   StringRef RepositoryUrl = "", StringRef Base = "") {
+  ClangDocContext CDCtx{{}, "test-project", {},   {},
+                        {}, RepositoryUrl,  Base, UserStylesheets};
   CDCtx.UserStylesheets.insert(
       CDCtx.UserStylesheets.begin(),
       "../share/clang/clang-doc-default-stylesheet.css");
@@ -299,7 +299,8 @@ TEST(HTMLGeneratorTest, emitFunctionHTML) {
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext({}, "https://www.repository.com");
+  ClangDocContext CDCtx =
+      getClangDocContext({}, "https://www.repository.com");
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(<!DOCTYPE html>

@github-actions
Copy link

github-actions bot commented Mar 21, 2025

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

@ilovepi ilovepi force-pushed the users/ilovepi/clang_doc_base_dir branch from 953bbdd to 1a9d876 Compare March 21, 2025 22:53
Currently, when we set URLs from JS, we set them only using the protocol
and host locations. This works fine when docs are served from the base
directory of the site, but if you want to nest it under another
directory, our JS fails to set the correct path, leading to broken
links.

This patch adds a --base option to specify the path prefix to use, which
is set in the generated index_json.js file. index.json can then fill in
the prefix appropriately when generating links in a browser. This flag
has no effect for non HTML output.

Given an index hosted at: www.docs.com/base_directory/index.html
we used to generate the following link:
    www.docs.com/file.html
Using --base base_directory we now generate:
    www.docs.com/base_directory/file.html

This allows such links to work when hosting pages without using a custom
index.js.
@ilovepi ilovepi force-pushed the users/ilovepi/clang_doc_base_dir branch from 1a9d876 to 4677619 Compare March 25, 2025 00:30
@@ -1,9 +1,10 @@
function genLink(Ref) {
Copy link
Member

Choose a reason for hiding this comment

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

It looks like we always invoke this function with two arguments: Index and CurrentDirectory, but we actually ignore the second argument and don't even specify it in the function signature. Do you know what is CurrentDirectory meant for? Could it perhaps be used instead of Base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like the current directory was left around when we moved from relative paths to absolute. I don't think we can use that instead of base, though.

When I log what the CurrentDirectory is on docs.fuchsia.dev/cpp using chrome tools, it ends up empty, so I think we may still need to use Base. i'll take another look at the JS though.

@petrhosek petrhosek requested a review from mysterymath March 26, 2025 18:17
Copy link
Contributor

@mysterymath mysterymath left a comment

Choose a reason for hiding this comment

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

LGTM for JS-isms

Copy link
Contributor Author

ilovepi commented Mar 28, 2025

Merge activity

  • Mar 28, 3:48 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Mar 28, 3:49 PM EDT: A user merged this pull request with Graphite.

@ilovepi ilovepi merged commit d4dc571 into main Mar 28, 2025
11 checks passed
@ilovepi ilovepi deleted the users/ilovepi/clang_doc_base_dir branch March 28, 2025 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants