-
Notifications
You must be signed in to change notification settings - Fork 654
Add function for escaping all markdown special characters #10478
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: dev
Are you sure you want to change the base?
Conversation
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.
@@ -12,6 +13,11 @@ namespace NuGet.Services.Messaging.Email | |||
/// </summary> | |||
public abstract class MarkdownEmailBuilder : EmailBuilder | |||
{ | |||
private static readonly IReadOnlyList<string> SpecialMarkdownChars = |
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.
We're doing kind of double for loop, could we do small perf improvement?
private static readonly HashSet<char> SpecialMarkdownChars = new()
{
'\\', '`', '*', '_', '{', '}', '[', ']', '<', '>', '(', ')', '#', '+', '-', '.', '!', '|'
};
@@ -40,5 +46,20 @@ protected override string GetHtmlBody() | |||
{ | |||
return Markdown.ToHtml(GetMarkdownBody()); | |||
} | |||
|
|||
public static string EscapeMarkdown(string text) |
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.
We can do single for loop.
public static string EscapeMarkdown(string text)
{
if (string.IsNullOrEmpty(text))
return text;
StringBuilder sb = new(text.Length + 10); // preallocate a bit more
foreach (char c in text)
{
if (SpecialMarkdownChars.Contains(c))
sb.Append('\\');
sb.Append(c);
}
return sb.ToString();
}
private static readonly IReadOnlyList<string> SpecialMarkdownChars = | ||
[ | ||
"\\", "`", "*", "_", "{", "}", "[", "]", "<", ">", "(", ")", "#", "+", "-", ".", "!", "|" | ||
]; |
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.
It would be great to include link where you got the list of special characters for the reference.
We aren't correctly escaping the markdown in several of the emails we send. This is especially problematic in emails which reference packages that contain underscores in the ID as the URLs we send are invalid (underscore in markdown is for italics). I fixed this issue in a few common emails, but didn't want to do a full pass as there are a lot of emails and that's out of scope for the specific issue being addressed.
Report Abuse
Before
After
Contact Support (own package)
Before
After
Upload Package
Before
After
Contact Package Owner
Before
After
Addresses #1635