tdumitrescu / virtual-jade

Compile Jade templates to Hyperscript for Virtual DOM libraries
Other
31 stars 4 forks source link

Automatic semicolon insertion #26

Closed nojvek closed 6 years ago

tdumitrescu commented 6 years ago

The semicolon insertion on L63 looks like it gives us something new (final semicolon for top-level code block), but for every other code case the semicolons already get inserted:

$ DEBUG=test npm test 2>&1 >/dev/null |grep "var baz ="
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
$ git checkout nojvek/nojvek-semicolons -- test
$ DEBUG=test npm test 2>&1 >/dev/null |grep "var baz ="
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;
        var baz = x + 1;

This is because of https://github.com/tdumitrescu/virtual-jade/blob/40c03bb92c813e77778a61e638a54ebdcb3d4fac/lib/compiler.js#L390.

Could you update your test cases so that they verify the difference to top-level code blocks? There's already a fixture at https://github.com/tdumitrescu/virtual-jade/blob/40c03bb92c813e77778a61e638a54ebdcb3d4fac/test/fixtures/top-level-code.jade

nojvek commented 6 years ago

Test cases are already fixed. I'm not sure what other test cases you need me to update ?

top level code lines

+    expect(js).to.contain(`var a = 1;\n`);
+    expect(js).to.contain(`var b = 2;\n`);

multiline code block

+    expect(js).to.contain(`foo = ['bar'];`);
+    expect(js).to.contain(`var z = [\n  1,\n  2,\n  3\n];`);
tdumitrescu commented 6 years ago

Sorry, I must have missed that you already touched the top-level code block test. LGTM!