feat: copy compare table as markdown#1533
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis PR introduces a new reusable Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
server/utils/docs/text.ts (1)
121-124: Avoid the non‑null assertion on array indexing.The guideline calls for explicit checks when indexing arrays; this avoids
!and keeps the code strictly type‑safe.♻️ Suggested fix
- for (let i = 0; i < codeBlockData.length; i++) { - const { lang, code } = codeBlockData[i]! + for (let i = 0; i < codeBlockData.length; i++) { + const entry = codeBlockData[i] + if (!entry) continue + const { lang, code } = entry const highlighted = await highlightCodeBlock(code, lang) result = result.replace(`__CODE_BLOCK_${i}__`, highlighted) }As per coding guidelines, "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
app/pages/compare.vue (1)
95-100: Consider adding a type guard for safer array access.Accessing
children[0]at line 97 could returnundefinedif the grid has no children. While line 100 handles the falsy case, TypeScript may not narrow the type correctly. The current logic works but could be made more explicit.Proposed defensive check
function gridToMarkdown(gridEl: HTMLElement): string { const children = Array.from(gridEl.children) - const headerRow = children[0] + const headerRow = children[0] as Element | undefined const dataRows = children.slice(1) if (!headerRow || dataRows.length === 0) return ''app/components/CopyToClipboardButton.vue (1)
51-83: Consider adding graceful degradation for older browsers withoutallow-discretesupport.The
allow-discretetransitions are well-supported across modern browsers (Chrome 117+, Safari 17.4+, Firefox 129+), but this is a feature projected to reach "Baseline widely available" by February 2027. For enhanced compatibility with older browser versions, consider either removingallow-discretefrom production builds or implementing a progressive enhancement strategy. Without it, transitions become discrete jumps rather than smooth animations—the component remains functional, but the visual refinement is lost.
danielroe
left a comment
danielroe
left a comment
There was a problem hiding this comment.
although these were already dependencies, I don't think any of them were used in the client.
I think there must be a better way to render the table to markdown without converting from html - as we have the source for that html, the most complex piece here would only be padding to ensure the columns are the same width
I see no issue with changing this and can update the code to just generate markdown from the client side data without doing the entire My idea was here to create a function we could re-use in the future while allowing this functionality as the first one using it. |
|
thank you ❤️
let's wait until we see if we need it in other cases - we can refer back to this PR for implementation if so. 🙏 |
|
100%, will update once I have a moment :) |
…with imports, reverted packages, new table to md function
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lunaria/files/en-US.json (1)
915-915: Optional: consider aligning the label with the existingcopy_as_markdownpattern.The sibling key
package.readme.copy_as_markdown(line 249) uses the value"Copy README as Markdown", which tells the user about the output format. The newcompare.packages.copy_as_markdownvalue"Copy table"omits any mention of Markdown, so users pasting into a plain-text editor may be surprised by the raw Markdown syntax.
"Copy table as Markdown"(or simply"Copy as Markdown") would be consistent with the existing pattern and make the output format explicit. This should be addressed in the basei18n/locales/en.jsonas well, sincelunaria/files/en-US.jsonmirrors it.
|
@danielroe updated Since you mentioned padding, do you think the link in the package headers should be removed? I padded only to names.
|
2 participants
Implements #1515
Packages used for html to markdown util are dependencies that already were used in the project.
How it work:
Screencast_20260216_192439.mp4