Apply fence lookahead optimisation for top-level code blocks#100
Conversation
Pull Request Test Coverage Report for Build b6d6dc090846f0ff9aefeba44dc524c140fd8255-PR-100Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| %Line.Fence{delimiter: delimiter, indent: 0} = fence when recursive -> | ||
| stop = ~r/\A (?: #{delimiter} ) \s* ([^`\s]*) \s* \z/xu | ||
| %Line.Fence{delimiter: delimiter, indent: 0} = fence -> | ||
| stop = ~r/\A (\s*) (?: #{delimiter} ) \s* ([^`\s]*) \s* \z/xu |
There was a problem hiding this comment.
The optional whitespace is necessary to handle code blocks is inside a list.
(Alternatively we could do this only when not recursive)
| messages = [{:warning, 1, "Failed to find closing <div>"}] | ||
|
|
||
| assert as_ast(markdown, annotations: @annotations) == {:ok, ast, messages} | ||
| assert as_ast(markdown, annotations: @annotations) == {:error, ast, messages} |
|
@jonatanklosko thanks for that, I am puzzled I played around with this during my current refactoring and it broke some cases, probably just me, I'll merge this and have a closer look. |
|
@RobertDober thanks! I realised we can easily test if the optimisation works as expected (#101), so we don't run into the issue again when refactoring :) |
|
@RobertDober I think when I first added this I also changed some tests, but they were corner cases that were not defined in the spec (iirc). |
|
@jonatanklosko and @josevalim thank you both for the code and your insights, I see now why this is ok and what I missunderstood... |
|
@RobertDober fantastic, thanks for releasing so quickly <3 |
4 participants
The optimisation was introduced in #70 then modified in #79, but with the latter it no longer applies to top-level code blocks (which is primarily what we need), this brings it back.