Add support for line numbers in Markdown code fences #1381
Conversation
coreyasmith
force-pushed
the
markdown-code-fence-line-numbers
branch
from
83e9ea2 to
1e99d12
Compare
WalkthroughThe pull request introduces a new function, ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/kg-markdown-html-renderer/test/markdown-html-renderer.test.js (1)
27-77: Consider adding edge case tests.While the current test coverage is good, consider adding tests for:
- Multiple language attributes
- Invalid
line-numbersplacement (as first word)- Empty info string
Example test cases:
it('handles multiple language attributes correctly', function () { const markdown = '```javascript experimental line-numbers\nconst foo = "bar";\n```'; const result = renderer.render(markdown, {ghostVersion: '4.0'}); result.should.containEql('class="line-numbers language-javascript"'); }); it('treats line-numbers as language when specified first', function () { const markdown = '```line-numbers javascript\nconst foo = "bar";\n```'; const result = renderer.render(markdown, {ghostVersion: '4.0'}); result.should.containEql('class="language-line-numbers"'); result.should.not.containEql('language-javascript'); }); it('handles empty info string correctly', function () { const markdown = '```\nconst foo = "bar";\n```'; const result = renderer.render(markdown, {ghostVersion: '4.0'}); result.should.not.containEql('line-numbers'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/kg-markdown-html-renderer/lib/markdown-html-renderer.js(2 hunks)packages/kg-markdown-html-renderer/test/markdown-html-renderer.test.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Node 22.13.1
- GitHub Check: Node 20.11.1
🔇 Additional comments (4)
packages/kg-markdown-html-renderer/lib/markdown-html-renderer.js (2)
45-72: LGTM! ThefenceLineNumbersfunction implementation is well-structured.The implementation correctly handles:
- Fallback to default behavior when no info string is present
- Proper parsing of language and attributes
- Validation of the
line-numbersattribute position- Graceful addition of the class attribute
The code is properly attributed to its original source.
109-110: LGTM! ThefenceLineNumbersplugin is correctly integrated.The plugin is appropriately added only for Ghost versions 4.x and later, maintaining backward compatibility.
packages/kg-markdown-html-renderer/test/markdown-html-renderer.test.js (2)
27-46: LGTM! The test cases for Ghost v4.x are comprehensive.The tests properly verify:
- Presence of
line-numbersclass when specified- Correct combination with language class
68-77: LGTM! The test case for Ghost <4.x ensures backward compatibility.The test verifies that
line-numbersis not included for older Ghost versions, maintaining backward compatibility.
Older versions of Ghost (2 and earlier?) used to support line numbers in Markdown code fences by adding
line-numbersafter the language:This would be rendered as:
The Prism Line Numbers plugin would then render line numbers for that code block.
This pull request adds in a markdown-it plugin that I implemented called markdown-it-fence-line-numbers to restore this functionality to Ghost's Markdown cards.