Skip to content

[clang-doc] [feat] add --repository-line-prefix argument #131280

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

Merged

Conversation

hulxv
Copy link
Member

@hulxv hulxv commented Mar 14, 2025

Description

This PR adds a new command-line option that allows users to specify the prefix used for line-based anchors in repository URLs. Different repository interfaces use different formats for line anchors (GitHub uses #L123, googlesource uses #123, etc.). This option enables users to customize the line prefix to match their repository platform without requiring hard-coded values for each service.

Fix #59814

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Mar 14, 2025

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

Author: Mohamed Emad (hulxv)

Changes

Description

This PR adds a new command-line option that allows users to specify the prefix used for line-based anchors in repository URLs. Different repository interfaces use different formats for line anchors (GitHub uses #L123, googlesource uses #<!-- -->123, etc.). This option enables users to customize the line prefix to match their repository platform without requiring hard-coded values for each service.

Fix #59814


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

6 Files Affected:

  • (modified) clang-tools-extra/clang-doc/HTMLGenerator.cpp (+41-25)
  • (modified) clang-tools-extra/clang-doc/MDGenerator.cpp (+6-1)
  • (modified) clang-tools-extra/clang-doc/Representation.cpp (+4)
  • (modified) clang-tools-extra/clang-doc/Representation.h (+4-1)
  • (modified) clang-tools-extra/clang-doc/tool/ClangDocMain.cpp (+9-4)
  • (added) clang-tools-extra/test/clang-doc/basic-project-with-line-prefix.test (+342)
diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
index 18a0de826630c..967275f93193b 100644
--- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -491,9 +491,9 @@ genReferencesBlock(const std::vector<Reference> &References,
   return Out;
 }
 
-static std::unique_ptr<TagNode>
-writeFileDefinition(const Location &L,
-                    std::optional<StringRef> RepositoryUrl = std::nullopt) {
+static std::unique_ptr<TagNode> writeFileDefinition(
+    const Location &L, std::optional<StringRef> RepositoryUrl = std::nullopt,
+    std::optional<StringRef> RepositoryLinePrefix = std::nullopt) {
   if (!L.IsFileInRootDir && !RepositoryUrl)
     return std::make_unique<TagNode>(
         HTMLTag::TAG_P, "Defined at line " + std::to_string(L.LineNumber) +
@@ -514,17 +514,21 @@ writeFileDefinition(const Location &L,
   Node->Children.emplace_back(std::make_unique<TextNode>("Defined at line "));
   auto LocNumberNode =
       std::make_unique<TagNode>(HTMLTag::TAG_A, std::to_string(L.LineNumber));
-  // The links to a specific line in the source code use the github /
-  // googlesource notation so it won't work for all hosting pages.
-  // FIXME: we probably should have a configuration setting for line number
-  // rendering in the HTML. For example, GitHub uses #L22, while googlesource
-  // uses #22 for line numbers.
-  LocNumberNode->Attributes.emplace_back(
-      "href", (FileURL + "#" + std::to_string(L.LineNumber)).str());
+
+  std::string LineAnchor = "#";
+
+  if (RepositoryLinePrefix)
+    LineAnchor += RepositoryLinePrefix.value().str();
+
+  LineAnchor += std::to_string(L.LineNumber);
+
+  LocNumberNode->Attributes.emplace_back("href", (FileURL + LineAnchor).str());
   Node->Children.emplace_back(std::move(LocNumberNode));
   Node->Children.emplace_back(std::make_unique<TextNode>(" of file "));
+
   auto LocFileNode = std::make_unique<TagNode>(
       HTMLTag::TAG_A, llvm::sys::path::filename(FileURL));
+
   LocFileNode->Attributes.emplace_back("href", std::string(FileURL));
   Node->Children.emplace_back(std::move(LocFileNode));
   return Node;
@@ -750,11 +754,15 @@ genHTML(const EnumInfo &I, const ClangDocContext &CDCtx) {
   Out.emplace_back(std::move(Table));
 
   if (I.DefLoc) {
-    if (!CDCtx.RepositoryUrl)
-      Out.emplace_back(writeFileDefinition(*I.DefLoc));
-    else
-      Out.emplace_back(
-          writeFileDefinition(*I.DefLoc, StringRef{*CDCtx.RepositoryUrl}));
+    std::optional<StringRef> RepoUrl;
+    std::optional<StringRef> RepoLinePrefix;
+
+    if (CDCtx.RepositoryUrl)
+      RepoUrl = StringRef{*CDCtx.RepositoryUrl};
+    if (CDCtx.RepositoryLinePrefix)
+      RepoLinePrefix = StringRef{*CDCtx.RepositoryLinePrefix};
+
+    Out.emplace_back(writeFileDefinition(*I.DefLoc, RepoUrl, RepoLinePrefix));
   }
 
   std::string Description;
@@ -799,11 +807,15 @@ genHTML(const FunctionInfo &I, const ClangDocContext &CDCtx,
   FunctionHeader->Children.emplace_back(std::make_unique<TextNode>(")"));
 
   if (I.DefLoc) {
-    if (!CDCtx.RepositoryUrl)
-      Out.emplace_back(writeFileDefinition(*I.DefLoc));
-    else
-      Out.emplace_back(writeFileDefinition(
-          *I.DefLoc, StringRef{*CDCtx.RepositoryUrl}));
+    std::optional<StringRef> RepoUrl;
+    std::optional<StringRef> RepoLinePrefix;
+
+    if (CDCtx.RepositoryUrl)
+      RepoUrl = StringRef{*CDCtx.RepositoryUrl};
+    if (CDCtx.RepositoryLinePrefix)
+      RepoLinePrefix = StringRef{*CDCtx.RepositoryLinePrefix};
+
+    Out.emplace_back(writeFileDefinition(*I.DefLoc, RepoUrl, RepoLinePrefix));
   }
 
   std::string Description;
@@ -866,11 +878,15 @@ genHTML(const RecordInfo &I, Index &InfoIndex, const ClangDocContext &CDCtx,
   Out.emplace_back(std::make_unique<TagNode>(HTMLTag::TAG_H1, InfoTitle));
 
   if (I.DefLoc) {
-    if (!CDCtx.RepositoryUrl)
-      Out.emplace_back(writeFileDefinition(*I.DefLoc));
-    else
-      Out.emplace_back(writeFileDefinition(
-          *I.DefLoc, StringRef{*CDCtx.RepositoryUrl}));
+    std::optional<StringRef> RepoUrl;
+    std::optional<StringRef> RepoLinePrefix;
+
+    if (CDCtx.RepositoryUrl)
+      RepoUrl = StringRef{*CDCtx.RepositoryUrl};
+    if (CDCtx.RepositoryLinePrefix)
+      RepoLinePrefix = StringRef{*CDCtx.RepositoryLinePrefix};
+
+    Out.emplace_back(writeFileDefinition(*I.DefLoc, RepoUrl, RepoLinePrefix));
   }
 
   std::string Description;
diff --git a/clang-tools-extra/clang-doc/MDGenerator.cpp b/clang-tools-extra/clang-doc/MDGenerator.cpp
index 7bef2c0db04d8..a3bade238635c 100644
--- a/clang-tools-extra/clang-doc/MDGenerator.cpp
+++ b/clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -57,7 +57,12 @@ static void writeFileDefinition(const ClangDocContext &CDCtx, const Location &L,
     OS << "*Defined at " << L.Filename << "#" << std::to_string(L.LineNumber)
        << "*";
   } else {
-    OS << "*Defined at [" << L.Filename << "#" << std::to_string(L.LineNumber)
+    OS << "*Defined at [" << L.Filename << "#";
+
+    if (!CDCtx.RepositoryLinePrefix) 
+      OS << StringRef{*CDCtx.RepositoryLinePrefix};  
+    
+    OS << std::to_string(L.LineNumber)
        << "](" << StringRef{*CDCtx.RepositoryUrl}
        << llvm::sys::path::relative_path(L.Filename) << "#"
        << std::to_string(L.LineNumber) << ")"
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
index 4da93b24c131f..74f867dfb9f0c 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -368,6 +368,7 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx,
                                  StringRef ProjectName, bool PublicOnly,
                                  StringRef OutDirectory, StringRef SourceRoot,
                                  StringRef RepositoryUrl,
+                                 StringRef RepositoryLinePrefix,
                                  std::vector<std::string> UserStylesheets)
     : ECtx(ECtx), ProjectName(ProjectName), PublicOnly(PublicOnly),
       OutDirectory(OutDirectory), UserStylesheets(UserStylesheets) {
@@ -381,6 +382,9 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx,
     if (!RepositoryUrl.empty() && !RepositoryUrl.starts_with("http://") &&
         !RepositoryUrl.starts_with("https://"))
       this->RepositoryUrl->insert(0, "https://");
+
+    if (!RepositoryLinePrefix.empty())
+      this->RepositoryLinePrefix = std::string(RepositoryLinePrefix);
   }
 }
 
diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h
index bb0c534af7b74..88b3e47dfb1ee 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 RepositoryCodeLinePrefix,
                   std::vector<std::string> UserStylesheets);
   tooling::ExecutionContext *ECtx;
   std::string ProjectName; // Name of project clang-doc is documenting.
@@ -518,10 +519,12 @@ struct ClangDocContext {
                             // the file is in this dir.
   // URL of repository that hosts code used for links to definition locations.
   std::optional<std::string> RepositoryUrl;
+  // Prefix of line code for repository.
+  std::optional<std::string> RepositoryLinePrefix;
   // Path of CSS stylesheets that will be copied to OutDirectory and used to
   // style all HTML files.
   std::vector<std::string> UserStylesheets;
-  // JavaScript files that will be imported in allHTML file.
+  // JavaScript files that will be imported in all HTML file.
   std::vector<std::string> JsScripts;
   Index Idx;
 };
diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
index 2ce707feb3d5e..237f60885b3dd 100644
--- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -99,6 +99,12 @@ URL of repository that hosts code.
 Used for links to definition locations.)"),
                   llvm::cl::cat(ClangDocCategory));
 
+static llvm::cl::opt<std::string>
+    RepositoryCodeLinePrefix("repository-line-prefix", llvm::cl::desc(R"(
+Prefix of line code for repository.
+)"),
+                             llvm::cl::cat(ClangDocCategory));
+
 enum OutputFormatTy {
   md,
   yaml,
@@ -141,8 +147,7 @@ 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;
+  for (DirIt DirStart = DirIt(UserAssetPath, FileErr), DirEnd;
        !FileErr && DirStart != DirEnd; DirStart.increment(FileErr)) {
     FilePath = DirStart->path();
     if (llvm::sys::fs::is_regular_file(FilePath)) {
@@ -268,8 +273,8 @@ Example usage for a project using a compile commands database:
       OutDirectory,
       SourceRoot,
       RepositoryUrl,
-      {UserStylesheets.begin(), UserStylesheets.end()}
-  };
+      RepositoryCodeLinePrefix,
+      {UserStylesheets.begin(), UserStylesheets.end()}};
 
   if (Format == "html") {
     if (auto Err = getHtmlAssetFiles(argv[0], CDCtx)) {
diff --git a/clang-tools-extra/test/clang-doc/basic-project-with-line-prefix.test b/clang-tools-extra/test/clang-doc/basic-project-with-line-prefix.test
new file mode 100644
index 0000000000000..b393e1d12dfba
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/basic-project-with-line-prefix.test
@@ -0,0 +1,342 @@
+// RUN: rm -rf %t && mkdir -p %t/docs %t/build
+// RUN: sed 's|$test_dir|%/S|g' %S/Inputs/basic-project/database_template.json > %t/build/compile_commands.json
+// RUN: clang-doc --format=html --output=%t/docs --executor=all-TUs %t/build/compile_commands.json --repository=https://repository.com --repository-line-prefix=LLL
+// RUN: FileCheck %s -input-file=%t/docs/index_json.js -check-prefix=JSON-INDEX
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/Shape.html -check-prefix=HTML-SHAPE
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/Calculator.html -check-prefix=HTML-CALC
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/Rectangle.html -check-prefix=HTML-RECTANGLE
+// RUN: FileCheck %s -input-file=%t/docs/GlobalNamespace/Circle.html -check-prefix=HTML-CIRCLE
+
+// JSON-INDEX: async function LoadIndex() {
+// JSON-INDEX-NEXT: return{
+// JSON-INDEX-NEXT:   "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT:   "Name": "",
+// JSON-INDEX-NEXT:   "RefType": "default",
+// JSON-INDEX-NEXT:   "Path": "",
+// JSON-INDEX-NEXT:   "Children": [
+// JSON-INDEX-NEXT:     {
+// JSON-INDEX-NEXT:       "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT:       "Name": "GlobalNamespace",
+// JSON-INDEX-NEXT:       "RefType": "namespace",
+// JSON-INDEX-NEXT:       "Path": "GlobalNamespace",
+// JSON-INDEX-NEXT:       "Children": [
+// JSON-INDEX-NEXT:         {
+// JSON-INDEX-NEXT:           "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT:           "Name": "Calculator",
+// JSON-INDEX-NEXT:           "RefType": "record",
+// JSON-INDEX-NEXT:           "Path": "GlobalNamespace",
+// JSON-INDEX-NEXT:           "Children": []
+// JSON-INDEX-NEXT:         },
+// JSON-INDEX-NEXT:         {
+// JSON-INDEX-NEXT:           "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT:           "Name": "Circle",
+// JSON-INDEX-NEXT:           "RefType": "record",
+// JSON-INDEX-NEXT:           "Path": "GlobalNamespace",
+// JSON-INDEX-NEXT:           "Children": []
+// JSON-INDEX-NEXT:         },
+// JSON-INDEX-NEXT:         {
+// JSON-INDEX-NEXT:           "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT:           "Name": "Rectangle",
+// JSON-INDEX-NEXT:           "RefType": "record",
+// JSON-INDEX-NEXT:           "Path": "GlobalNamespace",
+// JSON-INDEX-NEXT:           "Children": []
+// JSON-INDEX-NEXT:         },
+// JSON-INDEX-NEXT:         {
+// JSON-INDEX-NEXT:           "USR": "{{([0-9A-F]{40})}}",
+// JSON-INDEX-NEXT:           "Name": "Shape",
+// JSON-INDEX-NEXT:           "RefType": "record",
+// JSON-INDEX-NEXT:           "Path": "GlobalNamespace",
+// JSON-INDEX-NEXT:           "Children": []
+// JSON-INDEX-NEXT:         }
+// JSON-INDEX-NEXT:       ]
+// JSON-INDEX-NEXT:     }
+// JSON-INDEX-NEXT:   ]
+// JSON-INDEX-NEXT: };
+// JSON-INDEX-NEXT: }
+
+//      HTML-SHAPE: <h1>class Shape</h1>
+// HTML-SHAPE-NEXT: <p>
+// HTML-SHAPE-NEXT: Defined at line
+// HTML-SHAPE-NEXT: <a href="https://repository.com/./include/Shape.h#LLL8">8</a>
+// HTML-SHAPE-NEXT: of file
+// HTML-SHAPE-NEXT: <a href="https://repository.com/./include/Shape.h">Shape.h</a>
+// HTML-SHAPE-NEXT: </p>
+//      HTML-SHAPE: <div>brief</div>
+//      HTML-SHAPE: <p> Abstract base class for shapes.</p>
+//      HTML-SHAPE: <p> Provides a common interface for different types of shapes.</p>
+//      HTML-SHAPE: <h2 id="Functions">Functions</h2>
+//      HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">area</h3>
+//      HTML-SHAPE: <p>public double area()</p>
+//      HTML-SHAPE: <div>brief</div>
+//      HTML-SHAPE: <p> Calculates the area of the shape.</p>
+//      HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">perimeter</h3>
+//      HTML-SHAPE: <p>public double perimeter()</p>
+//      HTML-SHAPE: <div>brief</div>
+//      HTML-SHAPE: <p> Calculates the perimeter of the shape.</p>
+//      HTML-SHAPE: <div>return</div>
+//      HTML-SHAPE: <p> double The perimeter of the shape.</p>
+//      HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">~Shape</h3>
+//      HTML-SHAPE: <p>public void ~Shape()</p>
+//      HTML-SHAPE: Defined at line 
+// HTML-SHAPE-NEXT: <a href="https://repository.com/./include/Shape.h#LLL13">13</a>
+// HTML-SHAPE-NEXT: of file 
+// HTML-SHAPE-NEXT: <a href="https://repository.com/./include/Shape.h">Shape.h</a>
+//      HTML-SHAPE: <div>brief</div>
+//      HTML-SHAPE: <p> Virtual destructor.</p>
+
+//      HTML-CALC: <h1>class Calculator</h1>
+// HTML-CALC-NEXT: <p>
+// HTML-CALC-NEXT: Defined at line 
+// HTML-CALC-NEXT: <a href="https://repository.com/./include/Calculator.h#LLL8">8</a>
+// HTML-CALC-NEXT: of file 
+// HTML-CALC-NEXT: <a href="https://repository.com/./include/Calculator.h">Calculator.h</a>
+// HTML-CALC-NEXT: </p>
+//      HTML-CALC: <div>brief</div>
+//      HTML-CALC: <p> A simple calculator class.</p>
+//      HTML-CALC: <p> Provides basic arithmetic operations.</p>
+//      HTML-CALC: <h2 id="Functions">Functions</h2>
+//      HTML-CALC: <h3 id="{{([0-9A-F]{40})}}">add</h3>
+//      HTML-CALC: <p>public int add(int a, int b)</p>
+//      HTML-CALC: Defined at line 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp#LLL3">3</a>
+// HTML-CALC-NEXT: of file 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp">Calculator.cpp</a>
+//      HTML-CALC: <div>brief</div>
+//      HTML-CALC: <p> Adds two integers.</p>
+//      HTML-CALC: <div>return</div>
+//      HTML-CALC: <p> int The sum of a and b.</p>
+//      HTML-CALC: <h3 id="{{([0-9A-F]{40})}}">subtract</h3>
+//      HTML-CALC: <p>public int subtract(int a, int b)</p>
+//      HTML-CALC: Defined at line 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp#LLL7">7</a>
+// HTML-CALC-NEXT: of file 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp">Calculator.cpp</a>
+//      HTML-CALC: <div>brief</div>
+//      HTML-CALC: <p> Subtracts the second integer from the first.</p>
+//      HTML-CALC: <div>return</div>
+//      HTML-CALC: <p> int The result of a - b.</p>
+//      HTML-CALC: <h3 id="{{([0-9A-F]{40})}}">multiply</h3>
+//      HTML-CALC: <p>public int multiply(int a, int b)</p>
+//      HTML-CALC: Defined at line 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp#LLL11">11</a>
+// HTML-CALC-NEXT: of file 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp">Calculator.cpp</a>
+//      HTML-CALC: <div>brief</div>
+//      HTML-CALC: <p> Multiplies two integers.</p>
+//      HTML-CALC: <div>return</div>
+//      HTML-CALC: <p> int The product of a and b.</p>
+//      HTML-CALC: <h3 id="{{([0-9A-F]{40})}}">divide</h3>
+//      HTML-CALC: <p>public double divide(int a, int b)</p>
+//      HTML-CALC: Defined at line 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp#LLL15">15</a>
+// HTML-CALC-NEXT: of file 
+// HTML-CALC-NEXT: <a href="https://repository.com/./src/Calculator.cpp">Calculator.cpp</a>
+//      HTML-CALC: <div>brief</div>
+//      HTML-CALC: <p> Divides the first integer by the second.</p>
+//      HTML-CALC: <div>return</div>
+//      HTML-CALC: <p> double The result of a / b.</p>
+//      HTML-CALC: <div>throw</div>
+//      HTML-CALC: <p>if b is zero.</p>
+
+//      HTML-RECTANGLE: <h1>class Rectangle</h1>
+// HTML-RECTANGLE-NEXT: <p>
+// HTML-RECTANGLE-NEXT: Defined at line
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./include/Rectangle.h#LLL10">10</a>
+// HTML-RECTANGLE-NEXT: of file 
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./include/Rectangle.h">Rectangle.h</a>
+// HTML-RECTANGLE-NEXT: </p>
+//      HTML-RECTANGLE: <p> Represents a rectangle with a given width and height.</p>
+//      HTML-RECTANGLE: <p>
+//      HTML-RECTANGLE:   Inherits from
+//      HTML-RECTANGLE:   <a href="Shape.html">Shape</a>
+//      HTML-RECTANGLE: </p>
+//      HTML-RECTANGLE: <h2 id="Members">Members</h2>
+//      HTML-RECTANGLE: <p> Width of the rectangle.</p>
+//      HTML-RECTANGLE: <div>private double width_</div>
+//      HTML-RECTANGLE: <p> Height of the rectangle.</p>
+//      HTML-RECTANGLE: <div>private double height_</div>
+//      HTML-RECTANGLE: <h2 id="Functions">Functions</h2>
+//      HTML-RECTANGLE: <h3 id="{{([0-9A-F]{40})}}">Rectangle</h3>
+//      HTML-RECTANGLE: <p>public void Rectangle(double width, double height)</p>
+//      HTML-RECTANGLE: Defined at line
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./src/Rectangle.cpp#LLL3">3</a>
+// HTML-RECTANGLE-NEXT: of file 
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./src/Rectangle.cpp">Rectangle.cpp</a>
+//      HTML-RECTANGLE: <div>brief</div>
+//      HTML-RECTANGLE: <p> Constructs a new Rectangle object.</p>
+//      HTML-RECTANGLE: <h3 id="{{([0-9A-F]{40})}}">area</h3>
+//      HTML-RECTANGLE: <p>public double area()</p>
+//      HTML-RECTANGLE: Defined at line
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./src/Rectangle.cpp#LLL6">6</a>
+// HTML-RECTANGLE-NEXT: of file 
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./src/Rectangle.cpp">Rectangle.cpp</a>
+//      HTML-RECTANGLE: <div>brief</div>
+//      HTML-RECTANGLE: <p> Calculates the area of the rectangle.</p>
+//      HTML-RECTANGLE: <div>return</div>
+//      HTML-RECTANGLE: <p> double The area of the rectangle.</p>
+//      HTML-RECTANGLE: <h3 id="{{([0-9A-F]{40})}}">perimeter</h3>
+//      HTML-RECTANGLE: <p>public double perimeter()</p>
+//      HTML-RECTANGLE: Defined at line
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./src/Rectangle.cpp#LLL10">10</a>
+// HTML-RECTANGLE-NEXT: of file 
+// HTML-RECTANGLE-NEXT: <a href="https://repository.com/./src/Rectangle.cpp">Rectangle.cpp</a>
+//      HTML-RECTANGLE: <div>brief</div>
+//      HTML-RECTANGLE: <p> Calculates the perimeter of the rectangle.</p>
+//      HTML-RECTANGLE: <div>return</div>
+//      HTML-RECTANGLE: <p> double The perimeter of the rectangle.</p>
+
+//      HTML-CIRCLE: <h1>class Circle</h1>
+// HTML-CIRCLE-NEXT: <p>
+// HTML-CIRCLE-NEXT: Defined at line 
+// HTML-CIRCLE-NEXT: <a href="https://repository.com/./include/Circle.h#LLL10">10</a>
+// HTML-CIRCLE-NEXT: of file 
+// HTML-CIRCLE-NEXT: <a href="https://repository.com/./include/Circle.h">Circle.h</a>
+// HTML-CIRCLE-NEXT: </p>
+//      HTML-CIRCLE: <div>brief</div>
+//      HTML-CIRCLE: <p> Circle class derived from Shape.</p>
+//      HTML-CIRCLE: <p> Represents a circle with a given radius.</p>
+//      HTML-CIRCLE: <p>
+//      HTML-CIRCLE:   Inherits from
+//      HTML-CIRCLE:   <a href="Shape.html">Shape</a>
+//      HTML-CIRCLE: </p>
+//      HTML-CIRCLE: <h2 id="Members">Members</h2>
+//      HTML...
[truncated]

@hulxv
Copy link
Member Author

hulxv commented Mar 14, 2025

@ilovepi Hey, could you please review this PR whenever you have a moment? I’d really appreciate your feedback!

@hulxv hulxv requested a review from ilovepi March 20, 2025 21:14
@hulxv
Copy link
Member Author

hulxv commented Mar 20, 2025

@ilovepi
About writeFileDefiniation, what's your opinion about making it similar to the markdown generator like this?

static void writeFileDefinition(const ClangDocContext &CDCtx, const Location &L);

I think it's more better

@ilovepi
Copy link
Contributor

ilovepi commented Mar 20, 2025

@ilovepi About writeFileDefiniation, what's your opinion about making it similar to the markdown generator like this?

static void writeFileDefinition(const ClangDocContext &CDCtx, const Location &L);

I think it's more better

That would probably be fine, except we do need some way to customize those strings. How do you envision the API would work if you remove those parameters?

@hulxv
Copy link
Member Author

hulxv commented Mar 21, 2025

That would probably be fine, except we do need some way to customize those strings. How do you envision the API would work if you remove those parameters?

Simply, it will work similarly to the markdown generator but without the raw stream

if (I.DefLoc)
writeFileDefinition(CDCtx, *I.DefLoc, OS);

static void writeFileDefinition(const ClangDocContext &CDCtx, const Location &L,
raw_ostream &OS) {
if (!CDCtx.RepositoryUrl) {
OS << "*Defined at " << L.Filename << "#" << std::to_string(L.LineNumber)
<< "*";
} else {
OS << "*Defined at [" << L.Filename << "#" << std::to_string(L.LineNumber)
<< "](" << StringRef{*CDCtx.RepositoryUrl}
<< llvm::sys::path::relative_path(L.Filename) << "#"
<< std::to_string(L.LineNumber) << ")"
<< "*";
}
OS << "\n\n";
}

@ilovepi
Copy link
Contributor

ilovepi commented Mar 21, 2025

Yeah that seems reasonable. You'll still want to check the validity of the optionals, which could make the logic a bit less nice, but overall I expect the code to be cleaner than what we have in a lot of places, right now.

ilovepi pushed a commit that referenced this pull request Mar 21, 2025
Fix the description typo for `JsScripts` in the context

discussed in #131280
@hulxv
Copy link
Member Author

hulxv commented Mar 22, 2025

@ilovepi done

Copy link

github-actions bot commented Mar 22, 2025

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

@hulxv hulxv force-pushed the clang-doc/fix/repository-doesnt-work-with-github branch 2 times, most recently from 48b0cdd to 357c0bd Compare March 24, 2025 01:26
@hulxv hulxv force-pushed the clang-doc/fix/repository-doesnt-work-with-github branch from 357c0bd to 02c9e07 Compare March 24, 2025 01:32
@hulxv hulxv requested a review from ilovepi March 24, 2025 20:43
@hulxv hulxv requested a review from ilovepi March 25, 2025 17:58
@ilovepi
Copy link
Contributor

ilovepi commented Mar 25, 2025

The patch is mostly in good shape. However, there is a build error. you can see it in the premerge CI. looks like you need to update the initialization of the clang doc context objects.

Also you need to git clang-format HEAD~ on your patch to get formatting check to pass. After that it will be ready to land.

@hulxv hulxv force-pushed the clang-doc/fix/repository-doesnt-work-with-github branch from 4d2ed22 to 54ede23 Compare March 25, 2025 20:23
@hulxv
Copy link
Member Author

hulxv commented Mar 25, 2025

Should I modify HTMLGeneratorTest.cpp to test RepositoryLinePrefix?

ClangDocContext CDCtx = getClangDocContext({}, "http://www.repository.com");

@ilovepi
Copy link
Contributor

ilovepi commented Mar 26, 2025

Should I modify HTMLGeneratorTest.cpp to test RepositoryLinePrefix?

ClangDocContext CDCtx = getClangDocContext({}, "http://www.repository.com");

You can, but please make that its own test.

@ilovepi
Copy link
Contributor

ilovepi commented Mar 26, 2025

The failures on the presubmit bot look unrelated, but you need to update your branch, since it looks like the base revision is broken.

@hulxv
Copy link
Member Author

hulxv commented Mar 26, 2025

The failures on the presubmit bot look unrelated, but you need to update your branch, since it looks like the base revision is broken.

Done

@hulxv
Copy link
Member Author

hulxv commented Mar 28, 2025

@ilovepi Ping

@ilovepi
Copy link
Contributor

ilovepi commented Mar 28, 2025

The branch has conflicts that need to be resolved.

While its not a problem, we typically wait for 1 week between reviews to "ping" a reviewer. https://llvm.org/docs/CodeReview.html#lgtm-how-a-patch-is-accepted

@hulxv
Copy link
Member Author

hulxv commented Mar 28, 2025

While its not a problem, we typically wait for 1 week between reviews to "ping" a reviewer. https://llvm.org/docs/CodeReview.html#lgtm-how-a-patch-is-accepted

Ok, I am sorry for that

@hulxv
Copy link
Member Author

hulxv commented Mar 28, 2025

The branch has conflicts that need to be resolved.

I think all is fine now

Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

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

LGTM, assuming CI passes.

@ilovepi
Copy link
Contributor

ilovepi commented Mar 28, 2025

If the premerge checks pass, then I'll land this for you.

@hulxv
Copy link
Member Author

hulxv commented Mar 28, 2025

It works without problems now😄

@ilovepi ilovepi merged commit 515d1ae into llvm:main Mar 28, 2025
11 checks passed
Copy link

@hulxv Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

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.

[clang-doc] --repository links don't work with github
3 participants