-
-
Notifications
You must be signed in to change notification settings - Fork 503
Support escaped quotes in generic attribute values #920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Add backslash escape handling for quoted attribute values following
CommonMark conventions. This allows including quote characters within
attribute values using backslash escapes (e.g. data-value="hello \"world\"").
This matches kramdown's behavior for inline attribute lists:
"If you need to use the value delimiter (a single or a double
quote) inside the value, you need to escape it with a backslash."
See: https://kramdown.gettalong.org/syntax.html
| line.SkipChar(); // Skip closing quote | ||
| c = line.CurrentChar; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| line.SkipChar(); // Skip closing quote | |
| c = line.CurrentChar; | |
| c = line.NextChar(); // Skip closing quote |
| // Parse a quoted string with backslash escape support (following CommonMark conventions) | ||
| if (c == '\'' || c == '"') | ||
| { | ||
| var valueBuilder = new ValueStringBuilder(stackalloc char[ValueStringBuilder.StackallocThreshold]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you can't stackalloc in a loop, this'll have to either be new ValueStringBuilder([]), or move the search logic into a separate helper method.
|
|
||
| properties ??= new(); | ||
| properties.Add(new KeyValuePair<string, string?>(name, valueBuilder.ToString())); | ||
| valueBuilder.Dispose(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| valueBuilder.Dispose(); |
(the ToString already disposes)
| properties ??= new(); | ||
| properties.Add(new KeyValuePair<string, string?>(name, valueBuilder.ToString())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it might be a bit easier to read if you change the structure to something like
string value;
if (c == '\'' || c == '"')
{
// ...
value = valueBuilder.ToString();
}
else
{
int start = line.Start;
// ...
value = slice.Text.Substring(...);
}
properties ??= new();
properties.Add(new KeyValuePair<string, string?>(name, value));There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds support for backslash-escaped quotes (and related escape sequences) inside quoted generic attribute values, enabling attribute strings that contain quote characters (e.g., Word field codes).
Changes:
- Update
GenericAttributesParserto parse quoted attribute values with backslash-escape handling. - Add spec cases covering escaped double/single quotes, escaped backslashes, non-escapable sequences, and a Word field-code example.
- Regenerate the corresponding spec-based NUnit test file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Markdig/Extensions/GenericAttributes/GenericAttributesParser.cs | Implements escape-aware parsing for quoted generic attribute values. |
| src/Markdig.Tests/Specs/GenericAttributesSpecs.md | Adds 6 spec examples documenting and validating escape behavior. |
| src/Markdig.Tests/Specs/GenericAttributesSpecs.generated.cs | Regenerated NUnit tests for the added spec examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| properties ??= new(); | ||
| properties.Add(new KeyValuePair<string, string?>(name, valueBuilder.ToString())); | ||
| valueBuilder.Dispose(); | ||
|
|
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ValueStringBuilder.ToString() already calls Dispose() (see Markdig.Helpers.ValueStringBuilder.ToString). The explicit valueBuilder.Dispose() right after valueBuilder.ToString() is redundant and can be removed to match existing usage patterns (e.g., HtmlHelper.TryParseHtmlTag).
| ## Escaped Quotes in Attribute Values | ||
|
|
||
| Backslash escapes are supported in quoted attribute values, following CommonMark conventions. | ||
| Escaped quotes (\") allow including quote characters within quoted values. |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this prose section, \" will be interpreted by Markdown and likely render without the backslash, which makes the documentation misleading. Consider using an inline code span (e.g., `\"`) or escaping the backslash (\\") so the rendered spec text shows the intended sequence.
| Escaped quotes (\") allow including quote characters within quoted values. | |
| Escaped quotes (`\"`) allow including quote characters within quoted values. |
Hi! I've been using Markdig and it's been great. Wanted to contribute back with this small feature.
This is my first PR here so happy to make any revisions or tweaks as necessary. And also, if this isn't something wanted by the maintainers, feel free to close.
Summary
data-value="hello \"world\"")Motivation
This was discovered when using markdown to generate Word documents from the Markdig AST. Word field codes use quoted strings:
This can work if the outer quotes are changed to single-quotes but escape sequences are useful for our context (and we preprocess the markdown so it works with Markdig today).
Implementation
Follows the same
hasEscapepattern used inLinkHelper.TryParseTitle, usingIsAsciiPunctuation()for escaping.This matches kramdown's behavior for inline attribute lists:
See: https://kramdown.gettalong.org/syntax.html
Test plan