Repository navigation
Formatting the GFM - #2
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR enhances OpenAPI file detection through content validation to prevent misclassification of generic JSON, improves error handling in markdown list parsing with explicit guards, switches markdown processing from async to synchronous execution, adjusts default quote preference to double quotes, and adds comprehensive test coverage for OpenAPI parsing, extensions API, and markdown formatting functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Detector as File Detector
participant Validator as Content Validator
participant Parser as OpenAPI Parser
Detector->>Detector: Check filepath for<br/>OpenAPI directory hint
alt Filepath matches pattern
Detector->>Validator: Validate content
alt Content has OpenAPI keys
Validator->>Parser: Accept as OpenAPI
Parser-->>Detector: isOpenAPI: true
else Generic JSON/YAML
Validator-->>Detector: Fallback required
Detector-->>Detector: Continue validation
end
else No filepath hint
Detector-->>Detector: Standard validation
end
sequenceDiagram
participant Text as Markdown Text
participant Parser as Parser Factory
participant Processor as Unified Processor
participant Output as AST Result
Text->>Parser: Input text
Parser->>Processor: Create processor with plugins
alt Before (Async)
Processor->>Processor: processor.run(parse)
Processor-->>Output: Promise + AST
end
alt After (Sync)
Processor->>Processor: processor.runSync(parse)
Processor-->>Output: Direct AST
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
package.json(2 hunks)src/index.ts(4 hunks)src/prettier-markdown/README.md(1 hunks)src/prettier-markdown/adapter-document-builders.js(1 hunks)src/prettier-markdown/adapter-document-constants.js(1 hunks)src/prettier-markdown/adapter-document-utils.js(1 hunks)src/prettier-markdown/adapter-pragma.js(1 hunks)src/prettier-markdown/adapter-prettier-internals.js(1 hunks)src/prettier-markdown/adapter-prettier-utils.js(1 hunks)src/prettier-markdown/clean.js(1 hunks)src/prettier-markdown/constants.evaluate.js(1 hunks)src/prettier-markdown/embed.js(1 hunks)src/prettier-markdown/format-markdown.js(1 hunks)src/prettier-markdown/format-markdown.ts(1 hunks)src/prettier-markdown/get-visitor-keys.js(1 hunks)src/prettier-markdown/index.js(1 hunks)src/prettier-markdown/languages.evaluate.js(1 hunks)src/prettier-markdown/loc.js(1 hunks)src/prettier-markdown/mdx.js(1 hunks)src/prettier-markdown/options.js(1 hunks)src/prettier-markdown/parser-markdown.js(1 hunks)src/prettier-markdown/pragma.js(1 hunks)src/prettier-markdown/print-paragraph.js(1 hunks)src/prettier-markdown/print-preprocess.js(1 hunks)src/prettier-markdown/print-sentence.js(1 hunks)src/prettier-markdown/print-whitespace.js(1 hunks)src/prettier-markdown/print/table.js(1 hunks)src/prettier-markdown/printer-markdown.js(1 hunks)src/prettier-markdown/unified-plugins/front-matter.js(1 hunks)src/prettier-markdown/unified-plugins/html-to-jsx.js(1 hunks)src/prettier-markdown/unified-plugins/liquid.js(1 hunks)src/prettier-markdown/unified-plugins/wiki-link.js(1 hunks)src/prettier-markdown/utils.js(1 hunks)src/prettier-markdown/visitor-keys.js(1 hunks)test/coverage.test.ts(1 hunks)test/extensions-api.test.ts(1 hunks)test/markdown-formatting.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (28)
src/prettier-markdown/index.js (3)
src/index.ts (2)
printers(309-320)parsers(298-307)src/prettier-markdown/format-markdown.js (1)
printer(9-9)src/prettier-markdown/printer-markdown.js (1)
printer(793-808)
src/prettier-markdown/print-sentence.js (2)
src/prettier-markdown/printer-markdown.js (10)
path(49-49)path(61-61)path(146-146)path(279-279)path(342-342)path(463-463)path(529-529)parts(70-70)parts(586-586)src/prettier-markdown/print-paragraph.js (2)
parts(17-17)parts(32-32)
src/prettier-markdown/format-markdown.js (5)
src/prettier-markdown/printer-markdown.js (10)
printer(793-808)index(506-506)path(49-49)path(61-61)path(146-146)path(279-279)path(342-342)path(463-463)path(529-529)doc(82-82)src/prettier-markdown/format-markdown.ts (1)
formatMarkdown(35-113)src/prettier-markdown/parser-markdown.js (2)
markdown(55-55)markdown(55-55)src/prettier-markdown/options.js (1)
options(4-18)src/prettier-markdown/adapter-prettier-internals.js (1)
getDocPrinter(103-129)
src/prettier-markdown/adapter-prettier-utils.js (1)
src/prettier-markdown/adapter-document-utils.js (3)
utils(38-38)utils(38-38)prettier(11-11)
src/prettier-markdown/print-whitespace.js (2)
src/prettier-markdown/utils.js (5)
i(152-152)KIND_CJ_LETTER(34-34)KIND_NON_CJK(33-33)KIND_K_LETTER(35-35)KIND_CJK_PUNCTUATION(36-36)src/prettier-markdown/printer-markdown.js (2)
value(280-281)proseWrap(148-152)
src/prettier-markdown/embed.js (3)
src/prettier-markdown/printer-markdown.js (12)
path(49-49)path(61-61)path(146-146)path(279-279)path(342-342)path(463-463)path(529-529)node(719-721)styleUnit(261-261)style(157-157)style(262-264)doc(82-82)src/prettier-markdown/adapter-prettier-utils.js (2)
getMaxContinuousCount(61-61)getMaxContinuousCount(61-61)src/prettier-markdown/format-markdown.js (2)
doc(85-91)ast(35-42)
src/prettier-markdown/print-preprocess.js (2)
src/prettier-markdown/format-markdown.js (1)
ast(35-42)src/prettier-markdown/utils.js (3)
text(184-187)text(189-191)i(152-152)
test/coverage.test.ts (1)
src/index.ts (2)
parsers(298-307)printers(309-320)
src/prettier-markdown/unified-plugins/front-matter.js (5)
src/prettier-markdown/unified-plugins/liquid.js (1)
proto(7-7)src/prettier-markdown/unified-plugins/wiki-link.js (1)
proto(9-9)src/prettier-markdown/printer-markdown.js (1)
value(280-281)src/prettier-markdown/adapter-pragma.js (1)
parseFrontMatter(40-40)src/prettier-markdown/pragma.js (1)
parseFrontMatter(18-18)
src/prettier-markdown/format-markdown.ts (3)
src/prettier-markdown/parser-markdown.js (2)
markdown(55-55)markdown(55-55)src/prettier-markdown/options.js (1)
options(4-18)src/prettier-markdown/adapter-prettier-internals.js (1)
getDocPrinter(103-129)
src/prettier-markdown/printer-markdown.js (6)
src/prettier-markdown/options.js (1)
options(4-18)src/prettier-markdown/utils.js (3)
node(222-222)INLINE_NODE_TYPES(5-24)INLINE_NODE_WRAPPER_TYPES(26-31)src/prettier-markdown/format-markdown.js (2)
doc(85-91)printer(9-9)src/prettier-markdown/constants.evaluate.js (1)
PUNCTUATION_REGEXP(75-84)src/prettier-markdown/adapter-prettier-utils.js (7)
getMinNotPresentContinuousCount(62-62)getMinNotPresentContinuousCount(62-62)getMaxContinuousCount(61-61)getMaxContinuousCount(61-61)UnexpectedNodeError(66-71)getPreferredQuote(63-63)getPreferredQuote(63-63)src/prettier-markdown/loc.js (2)
locStart(1-1)locEnd(2-2)
src/prettier-markdown/adapter-document-builders.js (6)
src/prettier-markdown/adapter-document-constants.js (1)
prettier(11-11)src/prettier-markdown/adapter-document-utils.js (1)
prettier(11-11)src/prettier-markdown/adapter-prettier-internals.js (5)
prettier(20-20)prettier(45-45)prettier(66-66)prettier(87-87)prettier(105-105)src/prettier-markdown/adapter-prettier-utils.js (1)
prettier(11-11)src/prettier-markdown/format-markdown.js (2)
require(10-10)doc(85-91)src/prettier-markdown/printer-markdown.js (1)
doc(82-82)
src/prettier-markdown/utils.js (2)
src/prettier-markdown/constants.evaluate.js (2)
CJK_REGEXP(23-26)PUNCTUATION_REGEXP(75-84)src/prettier-markdown/loc.js (2)
locStart(1-1)locEnd(2-2)
src/prettier-markdown/unified-plugins/html-to-jsx.js (2)
src/prettier-markdown/utils.js (1)
INLINE_NODE_WRAPPER_TYPES(26-31)src/prettier-markdown/mdx.js (1)
COMMENT_REGEX(28-28)
src/prettier-markdown/pragma.js (2)
src/prettier-markdown/adapter-pragma.js (8)
hasPragma(30-32)parseFrontMatter(40-40)MARKDOWN_HAS_PRAGMA_REGEXP(9-9)hasIgnorePragma(34-37)MARKDOWN_HAS_IGNORE_PRAGMA_REGEXP(10-10)insertPragma(39-45)pragma(41-41)FORMAT_PRAGMA_TO_INSERT(11-11)src/prettier-markdown/unified-plugins/front-matter.js (2)
parseFrontMatter(14-14)frontMatter(8-21)
src/prettier-markdown/unified-plugins/wiki-link.js (2)
src/prettier-markdown/unified-plugins/front-matter.js (1)
proto(9-9)src/prettier-markdown/unified-plugins/liquid.js (3)
proto(7-7)methods(8-8)match(13-13)
src/prettier-markdown/clean.js (1)
src/prettier-markdown/adapter-pragma.js (1)
hasPragma(30-32)
src/prettier-markdown/print/table.js (1)
src/prettier-markdown/options.js (1)
options(4-18)
src/prettier-markdown/print-paragraph.js (3)
src/prettier-markdown/printer-markdown.js (11)
path(49-49)path(61-61)path(146-146)path(279-279)path(342-342)path(463-463)path(529-529)parts(70-70)parts(586-586)doc(82-82)src/prettier-markdown/print-sentence.js (1)
parts(17-17)src/prettier-markdown/adapter-document-builders.js (1)
doc(23-23)
src/prettier-markdown/parser-markdown.js (4)
src/prettier-markdown/mdx.js (2)
BLOCKS_REGEX(27-27)esSyntax(71-81)src/prettier-markdown/unified-plugins/front-matter.js (1)
frontMatter(8-21)src/prettier-markdown/unified-plugins/liquid.js (1)
liquid(6-25)src/prettier-markdown/unified-plugins/wiki-link.js (1)
wikiLink(6-30)
src/prettier-markdown/get-visitor-keys.js (1)
src/prettier-markdown/visitor-keys.js (1)
visitorKeys(1-39)
test/extensions-api.test.ts (1)
src/extensions/index.ts (5)
isValidExtensionKey(894-896)createContextExtensions(865-870)before(961-964)after(966-969)createPositionHelpers(908-917)
src/index.ts (2)
src/prettier-markdown/index.js (2)
printers(3-5)printers(3-5)src/prettier-markdown/format-markdown.js (2)
trimmed(28-28)require(10-10)
src/prettier-markdown/adapter-pragma.js (1)
src/prettier-markdown/unified-plugins/front-matter.js (2)
parseFrontMatter(14-14)frontMatter(8-21)
src/prettier-markdown/unified-plugins/liquid.js (2)
src/prettier-markdown/unified-plugins/front-matter.js (1)
proto(9-9)src/prettier-markdown/unified-plugins/wiki-link.js (3)
proto(9-9)methods(10-10)match(15-15)
src/prettier-markdown/languages.evaluate.js (1)
src/index.ts (1)
languages(285-296)
src/prettier-markdown/adapter-prettier-internals.js (5)
src/prettier-markdown/adapter-document-builders.js (1)
prettier(11-11)src/prettier-markdown/adapter-document-constants.js (1)
prettier(11-11)src/prettier-markdown/adapter-document-utils.js (2)
prettier(11-11)docUtils(20-20)src/prettier-markdown/adapter-prettier-utils.js (1)
prettier(11-11)src/prettier-markdown/format-markdown.js (1)
require(10-10)
src/prettier-markdown/adapter-document-utils.js (2)
src/prettier-markdown/adapter-prettier-internals.js (6)
prettier(20-20)prettier(45-45)prettier(66-66)prettier(87-87)prettier(105-105)docUtils(117-117)src/prettier-markdown/format-markdown.js (1)
require(10-10)
🪛 LanguageTool
src/prettier-markdown/README.md
[uncategorized] ~3-~3: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ion This directory contains Prettier's markdown language implementation, adapted for us...
(MARKDOWN_NNP)
[uncategorized] ~27-~27: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ....ts- Type-safe wrapper for formatting markdown -options.js` - Markdown formatting op...
(MARKDOWN_NNP)
[uncategorized] ~32-~32: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...rom Prettier When Prettier updates its markdown implementation: 1. **Copy updated file...
(MARKDOWN_NNP)
[uncategorized] ~47-~47: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...te adapter-pragma.js 4. Test that markdown formatting still works correctly ## De...
(MARKDOWN_NNP)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
src/prettier-markdown/unified-plugins/wiki-link.js (1)
7-29: Tokenizer registration looks solid. Inserting the wiki-link tokenizer ahead of the corelinktokenizer ensures priority while keeping the locator lightweight. Nicely done.src/prettier-markdown/print-paragraph.js (1)
16-53: Paragraph flattening logic reads clean. Recursing through nested array/fill docs before handing them tofillmirrors Prettier’s internal approach and should maintain the odd-length invariant—looks good.
| const doc = require("prettier/doc"); | ||
| if (doc) { | ||
| builders = doc; | ||
| } |
There was a problem hiding this comment.
Fix fallback assignment when loading prettier/doc
require("prettier/doc") returns the doc namespace (with a builders property), not the builders themselves. Assigning builders = doc leaves every exported builder undefined, so any downstream call to group, line, etc. will crash. Please grab doc.builders instead.
- const doc = require("prettier/doc");
- if (doc) {
- builders = doc;
- }
+ const doc = require("prettier/doc");
+ if (doc?.builders) {
+ builders = doc.builders;
+ }🤖 Prompt for AI Agents
In src/prettier-markdown/adapter-document-builders.js around lines 23 to 26, the
fallback assigns the entire module namespace to builders which leaves exported
builder functions undefined; change the assignment to use the builders property
from the required module (i.e. set builders = doc.builders) so downstream calls
like group/line reference the actual functions.
| const contents = path.map( | ||
| () => | ||
| path.map(({ index: columnIndex }) => { | ||
| const text = printDocToString(print(), options).formatted; | ||
| const width = getStringWidth(text); | ||
| columnMaxWidths[columnIndex] = Math.max( | ||
| columnMaxWidths[columnIndex] ?? 3, // minimum width = 3 (---, :--, :-:, --:) | ||
| width, | ||
| ); | ||
| return { text, width }; | ||
| }, "children"), | ||
| "children", |
There was a problem hiding this comment.
Fix undefined column indices in printTable.
path.map passes the child path as the first argument and the numeric index as the second. The destructuring ({ index: columnIndex }) therefore leaves columnIndex as undefined, so we store widths under the "undefined" key. Later, columnMaxWidths[realIndex] stays undefined, spaces becomes NaN, and ' '.repeat(NaN) throws RangeError: Invalid count value when formatting any table. Please pull the numeric index from the callback parameter and hand the column path to print() directly.
- const contents = path.map(
- () =>
- path.map(({ index: columnIndex }) => {
- const text = printDocToString(print(), options).formatted;
+ const contents = path.map(
+ (rowPath) =>
+ rowPath.map((columnPath, columnIndex) => {
+ const text = printDocToString(print(columnPath), options).formatted;
const width = getStringWidth(text);
columnMaxWidths[columnIndex] = Math.max(
columnMaxWidths[columnIndex] ?? 3, // minimum width = 3 (---, :--, :-:, --:)
width,
);
return { text, width };
}, "children"),
"children",
);🤖 Prompt for AI Agents
In src/prettier-markdown/print/table.js around lines 16–27, the inner path.map
is destructuring the first parameter so columnIndex becomes undefined; change
the callback signature to accept (child, columnIndex) and call print(child) (or
print(child) via print on the child path) so you actually receive the numeric
index, then use that columnIndex when computing/storing columnMaxWidths and
widths; this ensures widths are stored under numeric keys and prevents
NaN/RangeError from ' '.repeat.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/index.ts (1)
80-95: Consider preserving parse error details for better diagnostics.The function currently returns
isOpenAPI: falsefor all parsing errors, making it impossible to distinguish between malformed YAML/JSON and valid files that aren't OpenAPI specs. Consider logging or propagating parse errors so users can diagnose syntax issues.Example approach:
} catch (error) { + // Log parse errors to help users diagnose malformed files + if (error instanceof Error) { + console.error(`Failed to parse file: ${error.message}`); + } return { isOpenAPI: false, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/index.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (2)
src/prettier-markdown/format-markdown.js (2)
trimmed(28-28)require(10-10)src/keys.ts (25)
InfoKeys(52-72)ComponentsKeys(89-122)OperationKeys(124-156)ParameterKeys(158-217)SchemaKeys(219-378)ResponseKeys(380-399)SecuritySchemeKeys(401-425)ServerKeys(441-453)TagKeys(467-476)ExternalDocsKeys(479-482)WebhookKeys(485-497)PathItemKeys(500-514)RequestBodyKeys(517-524)MediaTypeKeys(528-533)EncodingKeys(535-546)HeaderKeys(548-587)LinkKeys(589-599)ExampleKeys(601-606)DiscriminatorKeys(609-612)XMLKeys(615-621)ContactKeys(76-80)LicenseKeys(82-86)OAuthFlowKeys(427-439)ServerVariableKeys(455-465)RootKeys(8-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (8)
src/index.ts (8)
1-46: LGTM: Clean imports and type definitions.The comprehensive key imports and new
formatMarkdownoption are well-structured. The type definitions correctly restrictformatto"json" | "yaml"and add the new markdown toggle.
305-305: Good fix: locEnd now returns correct length.This addresses the previous review comment about range formatting. The locEnd now correctly returns
text.lengthinstead ofnode.content?.length, which should make Prettier's range calculations work properly.
328-361: Markdown formatting implementation has changed since previous review.Note that the previous Cursor review flagged missing Prettier APIs, but the current implementation uses a custom module (
./prettier-markdown/format-markdown.js) added in this PR rather than Prettier internals. The fallback to trimmed markdown on error provides a reasonable safety net.
399-424: LGTM: Proper markdown integration and YAML configuration.The formatting pipeline (sort → format markdown → serialize) is logical. The
lineWidth: -1for YAML correctly prevents line wrapping that would break markdown formatting.
426-532: Complex but necessary sorting logic with good prioritization.The multi-level detection (checking specific types before falling back to generic detection) is appropriate given overlapping properties in OpenAPI objects. The special handling for parameters (line 496-505) to sort
$refitems first is a nice UX improvement.
569-740: Well-designed detection functions with intentional overlap.The apparent overlap in property checks (e.g.,
descriptionappears in multiple functions) is intentional and reflects the OpenAPI spec where different object types share properties. The priority ordering insortOpenAPIKeys(lines 435-450) correctly disambiguates. The strict checks inisSchemaObject(lines 597-622) help prevent false positives.
752-808: Sophisticated key sorting with proper extension interleaving.The unified sorting function correctly handles the three-way ordering (standard keys, custom extensions, unknown keys) with proper relative positioning. The logic for interleaving custom extensions at specific positions (lines 776-796) is complex but necessary for flexible extension placement.
813-955: Comprehensive context detection with good Swagger 2.0 support.The path-first detection strategy (lines 815-866) is more reliable than pure object inspection. The defensive check at line 872 (only checking
isOperationObjectif path is non-empty) prevents false positives at the root level. The mapping of Swagger 2.0 fields (lines 949-951) ensures backward compatibility.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/prettier-markdown/parser-markdown.js (1)
27-43: Parser correctly uses synchronous API.The parser properly uses
processor.runSync(processor.parse(text))to maintain Prettier's synchronous contract. The previous async issue has been resolved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/prettier-markdown/parser-markdown.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/prettier-markdown/parser-markdown.js (4)
src/prettier-markdown/mdx.js (2)
BLOCKS_REGEX(27-27)esSyntax(71-81)src/prettier-markdown/unified-plugins/front-matter.js (1)
frontMatter(8-21)src/prettier-markdown/unified-plugins/liquid.js (1)
liquid(6-25)src/prettier-markdown/unified-plugins/wiki-link.js (1)
wikiLink(6-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (5)
src/prettier-markdown/parser-markdown.js (5)
1-11: LGTM!The imports are well-organized and properly structured for the unified/remark pipeline.
29-40: LGTM!The unified processor pipeline is well-structured with appropriate plugin ordering and conditional MDX support via the
isMDX ? plugin : nooppattern.
46-46: LGTM!The
noophelper is appropriately used as a no-op fallback for conditional plugin inclusion in the unified pipeline.
48-54: LGTM!The
baseParserconfiguration correctly defines the shared parser properties required by Prettier's parser contract.
56-58: LGTM!The parser exports are cleanly structured, with both
markdownandmdxparsers properly configured, andremarkprovided as a convenient alias.
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/prettier-markdown/utils.js (1)
103-103: Replace.at(-1)for Node 14 compatibility.This line still uses
innerToken.at(-1), which isn't available on Node 14.21.3 that Prettier 3 supports.Apply this diff:
- hasTrailingPunctuation: PUNCTUATION_REGEXP.test(innerToken.at(-1)), + hasTrailingPunctuation: PUNCTUATION_REGEXP.test(innerToken[innerToken.length - 1]),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/prettier-markdown/utils.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/prettier-markdown/utils.js (3)
src/prettier-markdown/constants.evaluate.js (2)
CJK_REGEXP(23-26)PUNCTUATION_REGEXP(75-84)src/prettier-markdown/format-markdown.js (1)
ast(35-42)src/prettier-markdown/loc.js (2)
locStart(1-1)locEnd(2-2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
src/prettier-markdown/utils.js (2)
150-158: Excellent defensive check for development.The non-production assertion effectively validates the splitText invariant without impacting production performance. This helps catch logic bugs early during development.
196-217: ****The nodes passed to
hasGitDiffFriendlyOrderedListcome from Prettier's remark-parsed markdown AST, which is pre-validated by the CommonMark-compliant parser. Ordered list items in the remark AST are structurally guaranteed to match the pattern expected bygetOrderedListItemInfo(number followed by.or)). Error handling is unnecessary here and would inappropriately hide real bugs. The function's assumption of valid input is correct for this internal code operating on pre-validated AST structures.Likely an incorrect or invalid review comment.
…t directories and update related tests for improved accuracy
…consistency in quote usage
Resolved conflicts: - adapter-prettier-utils.js: Kept HEAD fix for getPreferredQuote (returns double quote when singleQuote is false) - parser-markdown.js: Kept HEAD runSync (synchronous processing) - utils.js: Kept main's .at(-1) and HEAD's null check fix for getOrderedListItemInfo - index.ts: Kept HEAD's content validation logic and locEnd implementation - test/markdown-formatting.test.ts: Kept HEAD's prettier import and helper functions
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/index.ts(3 hunks)src/prettier-markdown/adapter-prettier-utils.js(1 hunks)src/prettier-markdown/parser-markdown.js(1 hunks)src/prettier-markdown/utils.js(1 hunks)test/coverage.test.ts(1 hunks)test/file-detection.test.ts(1 hunks)test/markdown-formatting.test.ts(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/coverage.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/prettier-markdown/parser-markdown.js
- src/prettier-markdown/utils.js
- src/prettier-markdown/adapter-prettier-utils.js
🧰 Additional context used
🧬 Code graph analysis (2)
test/file-detection.test.ts (1)
src/index.ts (1)
parsers(324-333)
test/markdown-formatting.test.ts (7)
src/prettier-markdown/format-markdown.js (3)
printer(9-9)require(10-10)doc(85-91)src/prettier-markdown/printer-markdown.js (2)
printer(793-808)doc(82-82)src/index.ts (1)
printers(335-346)src/prettier-markdown/adapter-document-constants.js (1)
prettier(11-11)src/prettier-markdown/adapter-document-builders.js (2)
prettier(11-11)doc(23-23)src/prettier-markdown/adapter-document-utils.js (2)
prettier(11-11)docUtils(20-20)src/prettier-markdown/adapter-prettier-internals.js (6)
prettier(20-20)prettier(45-45)prettier(66-66)prettier(87-87)prettier(105-105)docUtils(117-117)
🪛 GitHub Actions: CI
src/index.ts
[error] 331-331: Type '(_node: OpenAPINode, text: string) => number' is not assignable to type '(node: any) => number'.
🪛 GitHub Check: Build & Test
src/index.ts
[failure] 331-331:
Type '(_node: OpenAPINode, text: string) => number' is not assignable to type '(node: any) => number'.
🔇 Additional comments (4)
src/index.ts (1)
139-185: LGTM! Content validation addresses previous false positive concern.The filepath-based detection now includes content safeguards to prevent generic JSON/YAML files in component directories from being misclassified as OpenAPI. The implementation correctly validates that content contains OpenAPI-specific properties before accepting it based on directory patterns.
test/file-detection.test.ts (1)
162-198: Excellent test coverage for content validation.The updated test suite properly validates the new behavior:
- Generic content in component directories is now correctly rejected
- Valid OpenAPI schema content in component directories is correctly accepted
These tests provide good coverage for the content validation changes in
src/index.tsand help prevent regression of the false positive issue.test/markdown-formatting.test.ts (2)
82-166: Tests correctly verify markdown formatting behavior.The assertions appropriately check for:
- Presence of formatted description fields
- Whitespace normalization (no 4+ consecutive newlines)
- Code block preservation with indentation
The test logic is sound.
168-258: Test coverage for nested objects and summary fields is appropriate.The tests correctly verify that markdown formatting is applied to:
- Description fields in nested structures (parameters, responses)
- Summary fields at various levels (info, operations)
The assertions properly check for the presence of formatted fields.
| // Helper function to convert Doc to formatted string using Prettier's printDocToString | ||
| function docToString(doc: any, printDocToString: any, options: any = {}): string { | ||
| // If already a string, return it | ||
| if (typeof doc === "string") { | ||
| return doc; | ||
| } | ||
|
|
||
| // If printDocToString is not available, try to stringify the doc | ||
| if (!printDocToString) { | ||
| // Fallback: if it's an object, try JSON.stringify or inspect | ||
| if (doc && typeof doc === "object") { | ||
| // Try to see if it has a toString that works | ||
| const str = String(doc); | ||
| if (str !== "[object Object]") { | ||
| return str; | ||
| } | ||
| // Otherwise, we need printDocToString | ||
| throw new Error("printDocToString is required to convert Doc to string"); | ||
| } | ||
| return String(doc); | ||
| } | ||
|
|
||
| // Use printDocToString to convert Doc to formatted string | ||
| const result = printDocToString(doc, { | ||
| printWidth: options.printWidth || 80, | ||
| tabWidth: options.tabWidth || 2, | ||
| useTabs: false, | ||
| }); | ||
|
|
||
| // printDocToString returns { formatted: string } | ||
| return result?.formatted || String(doc); | ||
| } |
There was a problem hiding this comment.
The fallback on line 79 may silently mask formatting failures.
If printDocToString doesn't return an object with a .formatted property (due to API changes or version incompatibility), the code falls back to String(doc), which will produce "[object Object]" for Doc objects—the exact issue the previous review flagged. This makes test assertions pass incorrectly.
Apply this diff to fail fast instead of masking the issue:
// Use printDocToString to convert Doc to formatted string
const result = printDocToString(doc, {
printWidth: options.printWidth || 80,
tabWidth: options.tabWidth || 2,
useTabs: false,
});
- // printDocToString returns { formatted: string }
- return result?.formatted || String(doc);
+ // printDocToString must return { formatted: string }
+ if (!result?.formatted) {
+ throw new Error("printDocToString did not return formatted string");
+ }
+ return result.formatted;🤖 Prompt for AI Agents
In test/markdown-formatting.test.ts around lines 49 to 80, the current fallback
returns String(doc) when printDocToString does not provide a .formatted property
which can silently mask formatting failures; change the behavior to fail fast by
detecting when result is null/undefined or lacks formatted and throw a clear
Error indicating printDocToString returned an unexpected shape (include the
actual result in the message) instead of returning String(doc), so tests will
surface API/version incompatibilities immediately.
…ng for performance
…d simplifying string conversion logic
1 participant
Description
Brief description of the changes in this PR.
Type of Change
Testing
Related Issues
Fixes #(issue number)
Additional Notes
Any additional information that reviewers should know.
Note
Optimizes markdown parsing/embedding and tightens OpenAPI file detection while adding extensive tests and minor utility fixes.
Written by Cursor Bugbot for commit c9b1a4b. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Bug Fixes
Tests