fix(docs): allow hyphens in fenced codeblock language (#2437)#2524
fix(docs): allow hyphens in fenced codeblock language (#2437)#2524MukundaKatta wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
The fenced-codeblock language regexes in render.ts and text.ts matched \w which excludes `-`, so languages like `glimmer-ts`, `vue-html`, and `objective-c` were truncated: the `-ts` suffix was stripped off the language and leaked into the top of the code body, breaking Shiki highlighting. Swap `\w` for `[\w-]` in the three affected regexes and add a regression test covering `glimmer-ts`. Fixes npmx-dev#2437
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated regex patterns in documentation parsing utilities to support hyphenated language identifiers (e.g. Changes
Suggested reviewers
✨ 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 |
|
Hello! Thank you for opening your first PR to npmx, @MukundaKatta! 🚀 Here’s what will happen next:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/utils/docs/render.ts (1)
194-196: Add a dedicated regression test for the@exampleparsing branch.These regex updates are in the
renderJsDocTagspath; please add a direct test that an@examplefenced block withglimmer-tsdoes not leak-tsinto the code body.As per coding guidelines "
**/*.{test,spec}.{ts,tsx}: Write unit tests for core functionality usingvitest".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/docs/render.ts` around lines 194 - 196, Add a vitest unit test (file matching **/*.{test,spec}.{ts,tsx}) that exercises the renderJsDocTags/example parsing branch in server/utils/docs/render.ts: construct a JSDoc with an `@example` fenced block using the language tag "glimmer-ts" and call the same parser path (the code that computes langMatch/lang/code around example.doc) or renderJsDocTags directly, then assert the returned lang remains "glimmer-ts" but the extracted code string does NOT contain the "-ts" suffix (i.e., the code should not include the language suffix); keep the test focused and titled to document the regression for "glimmer-ts" example parsing.test/unit/server/utils/docs/text.spec.ts (1)
296-305: Strengthen this regression by asserting the highlight path explicitly.Add a positive check for Shiki output so this test also guarantees
glimmer-tsis parsed and highlighted, not just that leakage is gone.Suggested test tweak
expect(result).not.toContain('-ts') + expect(result).toContain('shiki') expect(result).toContain('const') expect(result).not.toContain('```')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/server/utils/docs/text.spec.ts` around lines 296 - 305, The test currently only checks that the "-ts" suffix doesn't leak into the code body; update the regression test that calls renderMarkdown(input, emptyLookup) to also assert that the Shiki highlight path was used by checking for Shiki-specific output (e.g. a language class or highlighted token spans) tied to the original fence language; in practice, after calling renderMarkdown in the 'should handle fenced code blocks with hyphenated language (e.g. glimmer-ts)' test, add a positive expect that the result contains the highlighting marker for glimmer-ts (for example a class attribute including "glimmer-ts" or Shiki token span markup) so you validate that renderMarkdown actually parsed and highlighted "glimmer-ts" rather than merely stripping the suffix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/utils/docs/render.ts`:
- Around line 194-196: Add a vitest unit test (file matching
**/*.{test,spec}.{ts,tsx}) that exercises the renderJsDocTags/example parsing
branch in server/utils/docs/render.ts: construct a JSDoc with an `@example` fenced
block using the language tag "glimmer-ts" and call the same parser path (the
code that computes langMatch/lang/code around example.doc) or renderJsDocTags
directly, then assert the returned lang remains "glimmer-ts" but the extracted
code string does NOT contain the "-ts" suffix (i.e., the code should not include
the language suffix); keep the test focused and titled to document the
regression for "glimmer-ts" example parsing.
In `@test/unit/server/utils/docs/text.spec.ts`:
- Around line 296-305: The test currently only checks that the "-ts" suffix
doesn't leak into the code body; update the regression test that calls
renderMarkdown(input, emptyLookup) to also assert that the Shiki highlight path
was used by checking for Shiki-specific output (e.g. a language class or
highlighted token spans) tied to the original fence language; in practice, after
calling renderMarkdown in the 'should handle fenced code blocks with hyphenated
language (e.g. glimmer-ts)' test, add a positive expect that the result contains
the highlighting marker for glimmer-ts (for example a class attribute including
"glimmer-ts" or Shiki token span markup) so you validate that renderMarkdown
actually parsed and highlighted "glimmer-ts" rather than merely stripping the
suffix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a4b62273-e81c-410a-8552-fa1a09573193
📒 Files selected for processing (3)
server/utils/docs/render.tsserver/utils/docs/text.tstest/unit/server/utils/docs/text.spec.ts
Summary
Fixes #2437.
The fenced-codeblock language regexes in
server/utils/docs/render.tsandserver/utils/docs/text.tsmatched\w, which doesn't include-. For Shiki languages with hyphens (glimmer-ts,vue-html,objective-c, etc.), the-suffixwas dropped from the captured language and leaked into the top of the code body — the code block rendered with broken highlighting and a stray-tsat the start.Swaps
\wfor[\w-]in the three affected regexes:render.ts— the two regexes that extractlangand strip the fence fromexample.doctext.ts— the fenced-block extraction inrenderMarkdownTest plan
test/unit/server/utils/docs/text.spec.tsforglimmer-tscovering both sides of the bug: the-tssuffix no longer leaks into the body, and highlighting kicks in.🤖 Generated with Claude Code