Skip to content

Conversation

@evelez7
Copy link
Member

@evelez7 evelez7 commented Nov 6, 2025

Text that is in between <pre> tags is formatted verbatim. Thus, the
text that was correctly indented in relation to its depth in HTML was
being indented incorrectly when rendered. That resulted in bad looking pages.

Copy link
Member Author

evelez7 commented Nov 6, 2025

@evelez7 evelez7 requested a review from ilovepi November 6, 2025 00:29
@evelez7 evelez7 marked this pull request as ready for review November 6, 2025 00:29
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2025

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

Author: Erick Velez (evelez7)

Changes

Text that is in between &lt;pre&gt; tags is formatted verbatim. Thus, the
text that was correctly indented in relation to its depth in HTML was
being indented incorrectly when rendered. That resulted in bad looking pages.


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

2 Files Affected:

  • (modified) clang-tools-extra/clang-doc/assets/class-template.mustache (+1-1)
  • (modified) clang-tools-extra/clang-doc/assets/namespace-template.mustache (+1-1)
diff --git a/clang-tools-extra/clang-doc/assets/class-template.mustache b/clang-tools-extra/clang-doc/assets/class-template.mustache
index b1a7470f7c33a..04f24e12374b2 100644
--- a/clang-tools-extra/clang-doc/assets/class-template.mustache
+++ b/clang-tools-extra/clang-doc/assets/class-template.mustache
@@ -142,7 +142,7 @@
                     {{#PublicMembers}}
                     <div id="{{Name}}" class="delimiter-container">
                         <pre>
-                            <code class="language-cpp code-clang-doc" >{{Type}} {{Name}}</code>
+<code class="language-cpp code-clang-doc" >{{Type}} {{Name}}</code>
                         </pre>
                         {{#MemberComments}}
                         <div>
diff --git a/clang-tools-extra/clang-doc/assets/namespace-template.mustache b/clang-tools-extra/clang-doc/assets/namespace-template.mustache
index d96bc5ce91f3a..d5836dcefbfc2 100644
--- a/clang-tools-extra/clang-doc/assets/namespace-template.mustache
+++ b/clang-tools-extra/clang-doc/assets/namespace-template.mustache
@@ -93,7 +93,7 @@
                             <li id="{{USR}}" style="max-height: 40px;">
                                 <a href="{{DocumentationFileName}}.html">
                                     <pre>
-                                        <code class="language-cpp code-clang-doc">class {{Name}}</code>
+<code class="language-cpp code-clang-doc">class {{Name}}</code>
                                     </pre>
                                 </a>
                             </li>

Copy link
Member Author

evelez7 commented Nov 6, 2025

See https://erickvelez.com/clang-doc-mustache-output/pr166672/ for display diff. The doc entities in the namespace page are no longer indented.

@ilovepi
Copy link
Contributor

ilovepi commented Nov 6, 2025

See https://erickvelez.com/clang-doc-mustache-output/pr166672/ for display diff. The doc entities in the namespace page are no longer indented.

What am I supposed to be looking at that is wrong? Are you saying there is extra indentation? The page looks OK to me. Or is that an example where its fixed? do you have an example where its wrong in that case?

@evelez7
Copy link
Member Author

evelez7 commented Nov 6, 2025

do you have an example where its wrong in that case?

It's fixed in the first link. See this link where you can see the indentation for all the classes in the global namespace.

https://erickvelez.com/clang-doc-mustache-output/pr164314/

If you go to a class, you'll also see that it's fixed for public members.

@ilovepi
Copy link
Contributor

ilovepi commented Nov 6, 2025

I guess that makes me wonder if we want a <pre><pre/> tag surrounding those, since they aren't really using any kind of special formatting for alignment or font.

Locally If I edit the HTML in DevTools, and remove those tags, it looks as expected. WDYT about that? I think its preferable to keep the template looking sane if we can.

Alternatively, we could tightly wrap the <pre> tags around the members, but that seems kind of hacky.

WDYT?

@evelez7
Copy link
Member Author

evelez7 commented Nov 6, 2025

I guess that makes me wonder if we want a \<pre\><pre/> tag surrounding those, since they aren't really using any kind of special formatting for alignment or font.

We need <pre> tags surrounding code blocks for highlight.js.

https://github.com/highlightjs/highlight.js?tab=readme-ov-file#in-the-browser

If want it done a different way, without needing the <pre> tags, we can explore that.

https://github.com/highlightjs/highlight.js?tab=readme-ov-file#using-custom-html

But I think the pre tags help to preserve line breaks and indentation in the code blocks themselves. So if someone uses a @code command in comments, the <pre> preserves their intended indentation and line breaks.

@evelez7
Copy link
Member Author

evelez7 commented Nov 6, 2025

We can try just putting them on one line, <pre><code ...> ... </code></pre> to reduce the depth.

@ilovepi
Copy link
Contributor

ilovepi commented Nov 6, 2025

I guess that makes me wonder if we want a \<pre\><pre/> tag surrounding those, since they aren't really using any kind of special formatting for alignment or font.

We need \<pre\> tags surrounding code blocks for highlight.js.

I think this probably makes sense for top level entities, like these type identifiers for the class and namespace.

https://github.com/highlightjs/highlight.js?tab=readme-ov-file#in-the-browser

If want it done a different way, without needing the \<pre\> tags, we can explore that.

https://github.com/highlightjs/highlight.js?tab=readme-ov-file#using-custom-html

The custom route is something I'd prefer to avoid if we can. It introduces a lot of complexity and extra JS to manage.

But I think the pre tags help to preserve line breaks and indentation in the code blocks themselves. So if someone uses a @code command in comments, the \<pre\> preserves their intended indentation and line breaks.

For general code blocks, sure, but I'm not sure how much we rely on it for formatting the top level identifiers, like what's in this patch. I guess, maybe its always fine to just wrap them "tightly" on a single line? I can't seem to think of a scenario where I'd expect that to do the wrong thing. Though, I'll admit I haven't thought of it too much, and I really need to see/play with concrete HTML to understand it personally.

@evelez7
Copy link
Member Author

evelez7 commented Nov 6, 2025

For general code blocks, sure, but I'm not sure how much we rely on it for formatting the top level identifiers, like what's in this patch.

It's true, for the top level entities all it does is highlight the tag keyword (class, union) and put it into a different font. We can easily emulate that with a CSS identifier for the keyword. also makes it customizable. We don't really need highlight.js to make that work.

@ilovepi
Copy link
Contributor

ilovepi commented Nov 6, 2025

Well, I think I'm fine w/ just always using the tight wrapping on <pre><code .../></pre> blocks. It does simplify our coloring schema a bit to just be consistent across the whole codebase.

If that looks OK to you, and the output looks as intended, lets just go w/ that, since its the minimal change I can think of

@evelez7 evelez7 force-pushed the users/evelez7/clang-doc-fix-bad-indentation branch from 8efcd30 to 9cfef46 Compare November 7, 2025 07:27
Text that is in between <pre> tags is formatted verbatim. Thus, the
text that was correctly indented in relation to its depth in HTML was
being indented incorrectly when rendered. That resulted in bad looking pages.
@evelez7 evelez7 force-pushed the users/evelez7/clang-doc-fix-bad-indentation branch from 9cfef46 to 7bbed06 Compare November 7, 2025 07:32
Copy link
Member Author

evelez7 commented Nov 7, 2025

Merge activity

  • Nov 7, 9:10 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Nov 7, 9:13 PM UTC: @evelez7 merged this pull request with Graphite.

@evelez7 evelez7 merged commit a7382f1 into main Nov 7, 2025
10 checks passed
@evelez7 evelez7 deleted the users/evelez7/clang-doc-fix-bad-indentation branch November 7, 2025 21:13
vinay-deshmukh pushed a commit to vinay-deshmukh/llvm-project that referenced this pull request Nov 8, 2025
Text that is in between `<pre>` tags is formatted verbatim. Thus, the
text that was correctly indented in relation to its depth in HTML was
being indented incorrectly when rendered. That resulted in bad looking pages.
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.

4 participants