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

Fixed #902: a bug of code block caused by span tags that cross multiple lines. #904

Closed
wants to merge 5 commits into
base: master
from

Conversation

5 participants
@xing5
Copy link

xing5 commented Nov 7, 2014

This patch makes sure all tags are enclosed in every line with the
correct syntax class.

Input:

/**
 * Establishes a contract for reading events stored in arbitrary formats from
 * reliable, resettable streams.
 */

Original output:

<div class="line"><span class="javadoc">/**</div>
<div class="line"> * Establishes a contract for reading events stored in arbitrary formats from</div>
<div class="line"> * reliable, resettable streams.</div>
<div class="line"> */</span></div>

With this patch:

<div class="line"><span class="javadoc">/**</span></div>
<div class="line"><span class="javadoc"> * Establishes a contract for reading events stored in arbitrary formats from</span></div>
<div class="line"><span class="javadoc"> * reliable, resettable streams.</span></div>
<div class="line"><span class="javadoc"> */</span></div>
Fixed a bug of code block caused by span tags that cross multiple lines.
This patch makes sure all tags are enclosed in every line with the
correct syntax class.

@xing5 xing5 referenced this pull request Nov 7, 2014

Closed

Code Block Rendering Problem #902

@xing5

This comment has been minimized.

Copy link
Author

xing5 commented Nov 9, 2014

After reading #795, I did some other changes, I can add these to this pull request if you guys agree.

@@ -88,8 +101,12 @@ module.exports = function(str, options){
     firstLine = options.first_line;

   lines.forEach(function(item, i){
-    numbers += '<div class="line">' + (i + firstLine) + '</div>';
-    content += '<div class="line">' + item + '</div>';
+    if (i !== 0) {
+      numbers += '\n';
+      content += '\n';
+    }
+    numbers += '<span class="line">' + (i + firstLine) + '</span>';
+    content += '<span class="line">' + item + '</span>';
   });

   var result = '<figure class="highlight' + (options.lang ? ' ' + options.lang : '') + '">' +

@xing5 xing5 referenced this pull request Dec 2, 2014

Closed

highlight render error #940

@mingming-killer

This comment has been minimized.

Copy link

mingming-killer commented on 6486fe3 Jan 29, 2015

good, the multi-line comment "/* */" is correct render in code block, thank you.

@tommy351 tommy351 closed this Nov 4, 2015

@may17

This comment has been minimized.

Copy link

may17 commented Feb 24, 2017

Hey guys. Is this issue really fixed? i still got a problem with multi line comments.
hexo version: 3.2.2
highlights.js version: 9.9

{% codeblock lang:javascript %}
/**
 * Test
 */
{% endcodeblock  %}
<pre>
  <div class="line">
    <span class="comment">/**
  </div>
  <div class="line">
    * Test
  </div>
  <div class="line">
      */</span>
   </div>
</pre>
@seaoak

This comment has been minimized.

Copy link
Member

seaoak commented Mar 1, 2017

Yes! The issue #902 and this pull request #904 are not fixed on current version of Hexo.

$ hexo version
hexo: 3.2.2
hexo-cli: 1.0.2
os: Linux 4.4.0-64-generic linux x64
http_parser: 2.7.0
node: 7.6.0
v8: 5.5.372.40
uv: 1.11.0
zlib: 1.2.11
ares: 1.10.1-DEV
modules: 51
openssl: 1.0.2k
icu: 58.2
unicode: 9.0
cldr: 30.0.3
tz: 2016j
$ grep 'version' node_modules/hexo-util/package.json
  "version": "0.6.0",
$

To fix this issue, modify hexo-util/lib/highlight.js:125
https://github.com/seaoak/hexo-util/blob/master/lib/highlight.js#L125

diff --git a/lib/highlight.js b/lib/highlight.js
index dcdd734..ff0ef2a 100644
--- a/lib/highlight.js
+++ b/lib/highlight.js
@@ -122,7 +122,17 @@ function highlight(str, options) {

 function tryHighlight(str, lang) {
   try {
-    return hljs.highlight(lang, str);
+    var matching = str.match(/(\r?\n)/);
+    var separator = matching ? matching[1] : '';
+    var lines = matching ? str.split(separator) : [str];
+    var result = hljs.highlight(lang, lines.shift());
+    var html = result.value + separator;
+    while (lines.length > 0) {
+      result = hljs.highlight(lang, lines.shift(), false, result.top);
+      html += result.value + separator;
+    }
+    result.value = html;
+    return result;
   } catch (err) {
     return;
   }

I will add some tests for this issue, and make a new pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment