Update to commonmark spec 0.29 #156
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…iples of 3 (spec 0.29)
* Allow spaces inside link destinations in pointy brackets * Disallow link destination beginning with `<` unless it is inside `<..>`
The spec was updated to clarify that they can also be part of setext headings. The way we make these work in our parser is to be able to "replace" the active paragraph block. In that case, we still need to collect link reference definitions from it. In order to implement this, I had to separate it from InlineParser. The new way is cleaner, and allows us to add a Node to the document as well (for #98). I think there's still room for improvement: We could parse the definition as we go in ParagraphParser and collect them earlier. That might eliminate the current "double parsing" that we do in some cases.
Hopefully this will allow us to reuse it for parsing link reference definitions in an incremental way. Also speeds up parsing a bit: -SpecBenchmark.parseExamples thrpt 50 453.493 ± 4.647 ops/s +SpecBenchmark.parseExamples thrpt 50 483.467 ± 3.418 ops/s
The old code would parse link reference definitions twice in the worst case. The new one parses it as part of paragraph parsing. Looks like this is faster too: -SpecBenchmark.parseExamples thrpt 50 485.743 ± 1.864 ops/s +SpecBenchmark.parseExamples thrpt 50 550.071 ± 8.638 ops/s -SpecBenchmark.parseWholeSpec thrpt 50 284.494 ± 2.641 ops/s +SpecBenchmark.parseWholeSpec thrpt 50 297.277 ± 3.272 ops/s
See commonmark/commonmark.js@c89b35c Also replaced the regex for escaping with a loop, which speeds up HTML rendering: -SpecBenchmark.parseAndRenderExamples thrpt 50 344.820 ± 1.215 ops/s +SpecBenchmark.parseAndRenderExamples thrpt 50 374.342 ± 2.445 ops/s -SpecBenchmark.parseAndRenderWholeSpec thrpt 50 151.209 ± 1.148 ops/s +SpecBenchmark.parseAndRenderWholeSpec thrpt 50 198.357 ± 2.601 ops/s (Note that these benchmarks include parsing, so rendering itself saw a very nice improvement.)
robinst
commented
Jul 12, 2019
| /** | ||
| * Parser for inline references | ||
| */ | ||
| public interface ReferenceParser { |
There was a problem hiding this comment.
FYI @lalunamel this has been removed (to be released). It shouldn't really impact you, but just a heads up.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was a larger one because of the change in link reference definitions, see commonmark/commonmark-spec#395. I also cleaned up parsing of definitions to avoid parsing them twice. We can now also add them as nodes to the document, which is nice.
Fixes #152.