BenchmarkDotNet
BenchmarkDotNet copied to clipboard
dotnet
fix Markdown output to escape pipe character
Hello! Looking to get my feet wet contributing to open source. I've made changes here that should fix #1839.
This will escape the pipe | character in markdown table cells by replacing it with \| so that the tables render properly.
Example before escaping:
And after:
@adamsitnik Appreciate the feedback!
BTW are there any other characters that should be escaped?
From what I've seen from testing, the pipe character is the only one that breaks output, but I will make another pass to confirm this after addressing your other comments later today.
@bakermo I think there should be EscapeSpecialCharacters too. Is it should escape all characters? \t too?
Try run this (\a is a sound character, and it makes sounds!):
public class Benchmarks
{
[Params("Shou\nld|Esca\ape")] public string Param;
[Benchmark] public bool Method() => Param is null;
}
@YegorStepanov Ah, good catch. Yes, I agree.
Do you think it would be best to have a single flag EscapeSpecialCharacters that includes the pipe, or keep them separate (EscapeSpecialCharacters and EscapePipe)?
My inclination is to keep them separate, as EscapeSpecialCharacters could handle all characters that we probably want escaped across multiple (or all) outputs, and leave EscapePipe specific to the outputs that care about it. That, or each export dialect defines its own character set that it needs escaped. Thoughts?
@bakermo My opinion is exactly the same as yours. Separate them.
I don't remember any other markdown-specific mistakes in the output, escapePipe is enough currently.
By implementing it you will close one or few other issues. You may find them and link to the PR.
The big refactoring will also fix raw markdown alignment when using bold text:
(the PR correctly escapes '|', the screenshots below from master branch)
Current:
Should be:
