Skip to content

Fix splitCodeIntoArray when it has nested spans + newlines#79

Merged
allejo merged 3 commits into
9.18from
hotfix/split-code-deeply-nested-spans
Oct 16, 2020
Merged

Fix splitCodeIntoArray when it has nested spans + newlines#79
allejo merged 3 commits into
9.18from
hotfix/split-code-deeply-nested-spans

Conversation

@allejo

@allejo allejo commented Oct 16, 2020

Copy link
Copy Markdown
Collaborator

The splitCodeIntoArray function was generating broken HTML when there were deeply nested \n characters because it wasn't closing the correct number of <span> tags.

Marking this as a draft to see what the performance impact is by using XPath.

Originally, splitCodeIntoArray was working off the assumption that <span>s would only nest one level deep when splitting on new lines. Notice the span.hljs-params element is a child to span.hljs-function. Because of this assumption, we were closing the hljs-params span but not the hljs-function span.

Output from highlight

<span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-title">QuoteEntity</span><span class="hljs-params">(
)</span></span>

Current splitCodeIntoArray output:

<span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-title">QuoteEntity</span><span class="hljs-params">(</span>
<span class="hljs-params">)</span></span>

What it should look like (fix in this PR):

<span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-title">QuoteEntity</span><span class="hljs-params">(</span></span>
<span class="hljs-function"><span class="hljs-params">)</span></span>

See westonruter/syntax-highlighting-code-block#193

@allejo

allejo commented Oct 16, 2020

Copy link
Copy Markdown
Collaborator Author

Ran some benchmarks on calling splitCodeIntoArray 200000 times on different code snippets and the differences in performance are negligible.

test_original             : 35.122 sec.
test_xpath                : 36.956 sec.


test_original             : 35.362 sec.
test_xpath                : 35.501 sec.


test_original             : 54.900 sec.
test_xpath                : 55.953 sec.


test_original             : 24.244 sec.
test_xpath                : 22.913 sec.

@allejo allejo marked this pull request as ready for review October 16, 2020 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant