Conversation
|
@matkoch |
There was a problem hiding this comment.
Pull request overview
Adds MDX-compatible snippet markers using Markdown link reference comment syntax so in-place snippet overwrites can work in JSX/MDX toolchains that don’t allow HTML comments.
Changes:
- Added parsing + rendering support for
[//]: # (snippet: ...)and[//]: # (web-snippet: ...)markers (preserving the marker format on overwrite). - Added InPlaceOverwrite tests for link-ref comment snippet markers (including missing-snippet output).
- Documented the new marker syntax in
readme.mdandreadme.source.md, and adjusted include handling to recognize the newendSnippetmarker.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tests/MarkdownProcessor/MarkdownProcessorTests.cs | Adds tests for link-ref comment snippet overwrite + missing behavior. |
| src/Tests/MarkdownProcessor/MarkdownProcessorTests.LinkRefComment_Overwrite*.verified.txt | Adds verified outputs for the new tests. |
| src/MarkdownSnippets/Processing/SnippetKey.cs | Introduces extractors and detectors for link-ref comment snippet + web-snippet lines. |
| src/MarkdownSnippets/Processing/MarkdownProcessor.cs | Integrates link-ref comment parsing into InPlaceOverwrite flow and preserves marker format on output. |
| src/MarkdownSnippets/Processing/IncludeProcessor.cs | Treats link-ref endSnippet as a “diff line” when injecting include markers. |
| readme.source.md / readme.md | Documents the MDX/link-ref comment syntax and examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static bool ShouldWriteIncludeOnDiffLine(string line) => | ||
| SnippetKey.IsSnippetLine(line) || | ||
| line.StartsWith("<!-- endSnippet -->") || | ||
| line.StartsWith("[//]: # (endSnippet)") || | ||
| line.EndsWith("```") || | ||
| line.StartsWith('|') || | ||
| line.EndsWith('|'); |
There was a problem hiding this comment.
ShouldWriteIncludeOnDiffLine checks snippet/end markers with StartsWith(...) on the raw line. Since snippet detection elsewhere trims leading whitespace, indented markers like [//]: # (endSnippet) (or <!-- endSnippet -->) won't match here and may cause include markers to be appended to the same line, potentially breaking subsequent snippet processing. Consider applying TrimStart() before these StartsWith checks (including the newly-added link-ref endSnippet check) to align behavior with SnippetKey.Extract* parsing.
| static bool ShouldWriteIncludeOnDiffLine(string line) => | |
| SnippetKey.IsSnippetLine(line) || | |
| line.StartsWith("<!-- endSnippet -->") || | |
| line.StartsWith("[//]: # (endSnippet)") || | |
| line.EndsWith("```") || | |
| line.StartsWith('|') || | |
| line.EndsWith('|'); | |
| static bool ShouldWriteIncludeOnDiffLine(string line) | |
| { | |
| var trimmed = line.TrimStart(); | |
| return | |
| SnippetKey.IsSnippetLine(trimmed) || | |
| trimmed.StartsWith("<!-- endSnippet -->") || | |
| trimmed.StartsWith("[//]: # (endSnippet)") || | |
| line.EndsWith("```") || | |
| trimmed.StartsWith('|') || | |
| line.EndsWith('|'); | |
| } |
| if (SnippetKey.ExtractLinkRefCommentWebSnippet(line, out url, out snippetKey, out viewUrl)) | ||
| { | ||
| AppendWebSnippet(url, snippetKey, viewUrl, useLinkRefFormat: true); | ||
|
|
||
| index++; | ||
|
|
||
| lines.RemoveUntil( | ||
| index, | ||
| "[//]: # (endSnippet)", | ||
| relativePath, | ||
| line); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Link-reference comment handling for web-snippets was added (ExtractLinkRefCommentWebSnippet / AppendWebSnippet(... useLinkRefFormat: true)), but there are no tests covering the overwrite and missing-snippet cases for the [//]: # (web-snippet: ...) format. Adding tests similar to WithCommentWebSnippetUpdate / LinkRefComment_Overwrite_Missing would help ensure the new marker format is preserved and endSnippet removal works as expected.
| public static bool ExtractLinkRefCommentSnippet(Line line, [NotNullWhen(true)] out string? key) | ||
| { | ||
| var lineCurrent = line.Current.AsSpan().TrimStart(); | ||
| if (!IsLinkRefCommentSnippetLine(lineCurrent)) | ||
| { | ||
| key = null; | ||
| return false; | ||
| } | ||
|
|
||
| // after "[//]: # (snippet: " = 18 chars | ||
| var substring = lineCurrent[18..]; | ||
| var indexOf = substring.IndexOf(")", StringComparison.Ordinal); |
There was a problem hiding this comment.
ExtractLinkRefCommentSnippet slices lineCurrent[18..] after only checking StartsWith("[//]: # (snippet:"). That prefix does not require a space after the colon, so inputs like [//]: # (snippet:myKey) (no space) will be mis-parsed (first character of the key is dropped). Consider deriving the start index from the actual prefix length and then trimming optional whitespace after the colon (or parsing by finding "snippet:" and the closing ) rather than using a hard-coded offset).
| // after "[//]: # (web-snippet: " = 22 chars | ||
| var substring = lineCurrent[22..]; |
There was a problem hiding this comment.
ExtractLinkRefCommentWebSnippet uses a hard-coded slice lineCurrent[22..] after checking StartsWith("[//]: # (web-snippet:"), which does not enforce a space after the colon. If a user writes [//]: # (web-snippet:https://...#key) the URL will be truncated by one character. Recommend parsing based on the actual prefix length and trimming optional whitespace, instead of relying on a fixed offset.
| // after "[//]: # (web-snippet: " = 22 chars | |
| var substring = lineCurrent[22..]; | |
| const string prefix = "[//]: # (web-snippet:"; | |
| var substring = lineCurrent[prefix.Length..].TrimStart(); |
#453