docs: prevent </dd> from breaking into newline in api.md #1738
Conversation
DivitJain26
requested review from
asyncapi-bot-eve,
derberg,
jonaslagoni and
magicmatatjahu
as code owners
|
WalkthroughDocumentation in apps/generator/docs/api.md was reformatted. A new Handlebars template for global index definition lists was added. The docs generation script in apps/generator/package.json now discovers partials via a glob pattern instead of an explicit list. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
What reviewer looks at during PR reviewThe following are ideal points maintainers look for during review. Reviewing these points yourself beforehand can help streamline the review process and reduce time to merge.
|
|
@DivitJain26 thanks for quick PR. Please just:
and can you specify how much |
DivitJain26
changed the title
|
is there something else I should do |
|
Hey @DivitJain26, I think you were on the right path, maybe
@derberg, I'm not sure how reliable None of the six I believe some other dmd partial(s) is/are generating these $ grep -r "<dd>" node_modules/dmd/partials/
node_modules/dmd/partials/main-index/module-index/module-index-dl.hbs:<dd>{{{md (inlineLinks description)}}}</dd>
node_modules/dmd/partials/main-index/global-index/global-index-dl.hbs:<dd>{{{md (inlineLinks description)}}}</dd>The
I think the issue is clearer now right? I think the These are nested inside subdirectories so what we can do to override these is:
and remove that "docs": "jsdoc2md --partial 'docs/jsdoc2md-handlebars/**/*.hbs' --files lib/generator.js > docs/api.md", |
I think only step 2 is enough, step 1 can be safely skipped for this issue. We can just have a |
DivitJain26
force-pushed
the
fix/1737-api-md-template
branch
from
58f8f7f to
fca8d11
Compare
Thank you so much @aasim-maverick, you have been a Great Help, @derberg Please review |
| {{/if~}} | ||
| <dt>{{>sig-link-html}}</dt> | ||
| <dd>{{{md (inlineLinks description)}}}</dd> | ||
| {{#if @last~}}</dl> |
There was a problem hiding this comment.
I'd suggest to put the </dl> on line 8 onto a newline on 9.
Maybe you missed this in your commit, did you? |
DivitJain26
force-pushed
the
fix/1737-api-md-template
branch
from
fca8d11 to
d1b9703
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/generator/docs/api.md (1)
69-88: Generated file reflects invalid HTML structure from template.This generated documentation contains invalid HTML where
<dl>elements wrap markdown-style list items. This is a consequence of the issue inparams-table.hbs(see my comment on that file).Once the template is fixed, regenerate this file to correct the HTML structure throughout the document.
The AI summary claims "purely presentational formatting changes" with "no functional logic changes," but the introduction of
<dl>wrapper around non-definition-list content is actually a structural HTML change that violates HTML5 specification.
🧹 Nitpick comments (1)
apps/generator/docs/api.md (1)
69-70: Consider using a proper heading for "Params" section.The static analysis tool (markdownlint) flags line 70 because
**Params**uses emphasis (bold) instead of a proper markdown heading. While this matches the existing template pattern, consider whether "Params" should be a heading (e.g.,### Params) for better document structure and accessibility.This would require updating the
params-table.hbstemplate to emit a heading instead of bold text. However, verify this doesn't conflict with the existing documentation structure and hierarchy.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
apps/generator/docs/api.md(10 hunks)apps/generator/docs/jsdoc2md-handlebars/main-index/global-index/global-index-dl.hbs(1 hunks)apps/generator/docs/jsdoc2md-handlebars/params-table.hbs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/generator/docs/jsdoc2md-handlebars/main-index/global-index/global-index-dl.hbs
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
apps/generator/docs/api.md
70-70: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
240-240: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
240-240: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
381-381: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
381-381: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
408-408: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
408-408: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
446-446: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
446-446: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
476-476: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
476-476: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
| <dl>{{#each this~}} | ||
| {{indent}}- {{name}}{{#if type}} {{>linked-type-list types=type.names delimiter=" | " }}{{/if}}{{#unless (equal defaultvalue undefined)}} {{>defaultvalue equals=true ~}}{{/unless}}{{#if description}} - {{{inlineLinks description}}}{{/if}} | ||
| {{/each}} | ||
| {{/each~}}</dl> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Invalid HTML structure: <dl> cannot contain markdown list items.
The current implementation wraps markdown-style list items (with - prefix) inside a <dl> element, which violates HTML5 specification. According to HTML5 standards, a <dl> (definition list) must contain only <dt> (definition term) and <dd> (definition description) elements, not raw text or markdown list items.
This malformed HTML will:
- Fail HTML validation
- Potentially break rendering in browsers
- Break accessibility tools and screen readers
- Not achieve the intended formatting
Based on the PR discussion, the root cause appears to be in the dmd partials (node_modules/dmd/partials/main-index/global-index/global-index-dl.hbs), not in this file. According to aasim-maverick's investigation, the solution should involve creating a local override of the dmd partial to control the whitespace around <dd> tags.
Recommended approach:
Either revert this change and implement the proper fix in the dmd partial overrides, or convert this template to generate proper HTML structure:
However, based on the PR discussion, please verify with the reviewer whether this file should be modified at all, or if the fix should be in the dmd partial overrides as suggested by aasim-maverick.
🏁 Script executed:
sed -n '1,20p' apps/generator/docs/jsdoc2md-handlebars/params-table.hbsLength of output: 334
Use <dt> and <dd> elements inside <dl>
The template currently nests markdown list items (- {{name}}…) inside <dl>, resulting in invalid HTML and broken rendering. Refactor to emit <dt>{{name}}</dt> and <dd>…</dd> pairs instead.
🤖 Prompt for AI Agents
In apps/generator/docs/jsdoc2md-handlebars/params-table.hbs around lines 4 to 6,
the template is outputting markdown list items inside a <dl>, producing invalid
HTML; replace the current list-item emit with proper definition term/description
pairs by rendering each parameter as <dt>{{name}}</dt> and a matching <dd>
containing the type, default value and description (preserving the existing
helpers: linked-type-list, defaultvalue and inlineLinks), ensuring conditional
rendering logic remains the same and keeping the surrounding <dl> wrapper.
|
I'm not sure why there's a large diff being formed in |
DivitJain26
force-pushed
the
fix/1737-api-md-template
branch
from
b9bd2bb to
7142aa2
Compare
DivitJain26
force-pushed
the
fix/1737-api-md-template
branch
from
7142aa2 to
08dd7a9
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/generator/docs/api.md(7 hunks)apps/generator/docs/jsdoc2md-handlebars/main-index/global-index/global-index-dl.hbs(1 hunks)apps/generator/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/generator/docs/jsdoc2md-handlebars/main-index/global-index/global-index-dl.hbs
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
apps/generator/docs/api.md
23-23: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
25-25: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
26-26: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
36-36: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
37-37: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
41-41: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
42-42: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
43-43: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
45-45: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
46-46: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
47-47: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
48-48: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
49-49: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
52-52: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
53-53: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
54-54: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
77-77: Bare URL used
(MD034, no-bare-urls)
344-344: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
344-344: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
371-371: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
371-371: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
408-408: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
408-408: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
437-437: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
437-437: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
| "test:integration:update": "npm run test:integration -- -u", | ||
| "test:cleanup": "rimraf \"test/temp\"", | ||
| "docs": "jsdoc2md --partial docs/jsdoc2md-handlebars/custom-sig-name.hbs docs/jsdoc2md-handlebars/main.hbs docs/jsdoc2md-handlebars/docs.hbs docs/jsdoc2md-handlebars/header.hbs docs/jsdoc2md-handlebars/defaultvalue.hbs docs/jsdoc2md-handlebars/link.hbs docs/jsdoc2md-handlebars/params-table.hbs --files lib/generator.js > docs/api.md", | ||
| "docs": "jsdoc2md --partial 'docs/jsdoc2md-handlebars/**/*.hbs' --files lib/generator.js > docs/api.md", |
There was a problem hiding this comment.
Fix docs script quoting for Windows compatibility.
Using single quotes in npm scripts keeps the quotes on Windows shells, so jsdoc2md receives the literal path 'docs/jsdoc2md-handlebars/**/*.hbs' and fails to load the partials. Switch to double quotes to keep the glob portable.
- "docs": "jsdoc2md --partial 'docs/jsdoc2md-handlebars/**/*.hbs' --files lib/generator.js > docs/api.md",
+ "docs": "jsdoc2md --partial \"docs/jsdoc2md-handlebars/**/*.hbs\" --files lib/generator.js > docs/api.md",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "docs": "jsdoc2md --partial 'docs/jsdoc2md-handlebars/**/*.hbs' --files lib/generator.js > docs/api.md", | |
| "docs": "jsdoc2md --partial \"docs/jsdoc2md-handlebars/**/*.hbs\" --files lib/generator.js > docs/api.md", |
🤖 Prompt for AI Agents
In apps/generator/package.json at line 21, the npm script uses single quotes
around the jsdoc2md partials glob which breaks on Windows; replace the single
quotes with double quotes (and escape them so the package.json remains valid
JSON) so the script reads the glob as "**/docs/jsdoc2md-handlebars/**/*.hbs**"
(i.e., change 'docs/jsdoc2md-handlebars/**/*.hbs' →
\"docs/jsdoc2md-handlebars/**/*.hbs\") to make the command portable across
shells.
| @@ -1,9 +1,3 @@ | |||
| --- | |||
There was a problem hiding this comment.
why are there so many changes in this file now?
4 participants
Description
</dd>being pushed to a new line in generatedapi.md.params-table.hbsto correctly inline the closing</dd>.docs/api.mdusing the updated template.Related issue(s)
Summary by CodeRabbit
Documentation
Chores