Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Markdown filter: failing existing test + code block newline problem #268

Closed
wants to merge 2 commits into from

2 participants

@RubenVerborgh

The Markdown filter test fails with Discount 2.1.0 because of newline differences.
Since it is difficult (and not functionally interesting) to try to reconcile the newline differences across different Markdown engines, I suggest to make the test newline-invariant.

Below is the failing test.

filters.test.js test :markdown filter: AssertionError: "<h1>foo</h1>\n\n<ul>\n<li>bar</li>\n<li>baz</li>\n</ul>\n" == "<h1>foo</h1>\n\n<ul><li>bar</li><li>baz</li></ul>"
    at /Users/ruben/Desktop/jade/test/filters.test.js:56:16
    at next (/Users/ruben/Library/Tools/node/lib/node/.npm/expresso/0.6.4/package/bin/expresso:769:25)
    at runSuite (/Users/ruben/Library/Tools/node/lib/node/.npm/expresso/0.6.4/package/bin/expresso:787:6)
    at check (/Users/ruben/Library/Tools/node/lib/node/.npm/expresso/0.6.4/package/bin/expresso:648:16)
    at runFile (/Users/ruben/Library/Tools/node/lib/node/.npm/expresso/0.6.4/package/bin/expresso:652:10)
    at Array.forEach (native)
    at runFiles (/Users/ruben/Library/Tools/node/lib/node/.npm/expresso/0.6.4/package/bin/expresso:629:13)
    at run (/Users/ruben/Library/Tools/node/lib/node/.npm/expresso/0.6.4/package/bin/expresso:598:5)
    at Object.<anonymous> (/Users/ruben/Library/Tools/node/lib/node/.npm/expresso/0.6.4/package/bin/expresso:851:13)
    at Module._compile (module.js:423:26)


   Failures: 1
@RubenVerborgh

Another problem is that markdown code blocks newlines are broken.

Input

:markdown
  y
      {
        x
          x
      }

Expected output

<p>y</p>
<pre><code>{
  x
    x
}
</code></pre>

Actual output

<p>y</p>
<pre><code>{
  x
    x

}
</code></pre>

Note the extra newline above the closing bracket. Since this is a pre block, this extra newline will show up in the output.

The problem is that Jade surrounds each indentation block by newlines. Since the second x starts a new indentation level, two newlines are written (one for each x).

The solution I'm proposing (which is far from ideal) is a distinct "filter" lexer mode in which filter contents are respected literally, and where a succession of outdents is interpreted as one large outdent. See current implementation, which passes all tests.

@RubenVerborgh RubenVerborgh reopened this
@RubenVerborgh

I also saw it's not just a problem of the Markdown filter; it also occurs when I try to add the same piece of code right into Jade code.

@tj
Owner
tj commented

This issue has been inactive for over 2 months so I'm closing it. If you think it's still an issue re-open. - tjbot

@tj tj closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 22 additions and 4 deletions.
  1. +9 −2 lib/parser.js
  2. +13 −2 test/filters.test.js
View
11 lib/parser.js
@@ -258,8 +258,10 @@ Parser.prototype = {
, attrs = this.accept('attrs');
this.lexer.pipeless = true;
+ this.lexer.inFilter = true;
block = this.parseTextBlock();
this.lexer.pipeless = false;
+ this.lexer.inFilter = false;
var node = new nodes.Filter(tok.val, block, attrs && attrs.attrs);
node.line = this.line();
@@ -332,7 +334,7 @@ Parser.prototype = {
*/
parseTextBlock: function(){
- var text = new nodes.Text;
+ var text = new nodes.Text, pendingIndent = false;
text.line = this.line();
var spaces = this.expect('indent').val;
if (null == this._spaces) this._spaces = spaces;
@@ -348,7 +350,10 @@ Parser.prototype = {
this.parseTextBlock().nodes.forEach(function(node){
text.push(node);
});
- text.push('\\n');
+ if(!this.lexer.inFilter)
+ text.push('\\n');
+ else
+ pendingIndent = true;
break;
default:
text.push(indent + this.advance().val);
@@ -357,6 +362,8 @@ Parser.prototype = {
if (spaces == this._spaces) this._spaces = null;
this.expect('outdent');
+ if(pendingIndent && 'outdent' != this.peek().type)
+ text.push('\\n');
return text;
},
View
15 test/filters.test.js
@@ -54,8 +54,19 @@ module.exports = {
'test :markdown filter': function(assert){
assert.equal(
- '<h1>foo</h1>\n\n<ul><li>bar</li><li>baz</li></ul>',
- render(':markdown\n #foo\n - bar\n - baz\n'))
+ '<h1>foo</h1><ul><li>bar</li><li>baz</li></ul>',
+ render(':markdown\n #foo\n - bar\n - baz\n').replace(/\n/g, ''))
+ },
+
+ 'test :markdown filter with preformatted code': function(assert){
+ assert.includes(
+ render([':markdown',
+ ' x',
+ ' {',
+ ' x',
+ ' x',
+ ' }.'].join('\n')),
+ '<pre><code>{\n x\n x\n}.\n</code></pre>')
},
'test :stylus filter': function(assert){
Something went wrong with that request. Please try again.