Skip to content
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

inline javascript gets confused by blank lines #1916

Closed
gilesbowkett opened this issue Apr 1, 2015 · 6 comments
Closed

inline javascript gets confused by blank lines #1916

gilesbowkett opened this issue Apr 1, 2015 · 6 comments

Comments

@gilesbowkett
Copy link

(Or, technically, the inline JavaScript feature gets confused when it encounters lines which are not blank, but which are preceded by dashes, and which represent blank lines of JavaScript.)

Everything's cool if I inline this code:

- function foo(bar) {
-   doStuff();
- }

But this code gets me dashes on my web page:

- function foo(bar) {
-
-   doStuff();
-
- }

The blank lines in the script have - and no spaces. You can use - plus any number of spaces as the start of a blank line, just not - on its own.

This also works:

- function foo(bar) {

-   doStuff();

- }
@gilesbowkett
Copy link
Author

also, this:

- for (var i = 0; i < 100; i++) {
-
-   doStuff(i);
-
- }

puts about 200 dashes in the output.

@meodai
Copy link

meodai commented Apr 1, 2015

for more than one line of JS it would be nicer to have something like #1919 anyway.

@TimothyGu
Copy link
Member

@gilesbowkett said:

Everything's cool if I inline this code:

- function foo(bar) {
-   doStuff();
- }

Actually, this is NOT supported. See #796. For example, the following would not work:

- function foo(bar) {
-   doStuff(
-   );
- }

You just got lucky that in your case every line can have a valid statement appended to it. See generated JS:

function foo(bar) {
jade_debug.shift();
jade_debug.unshift({ lineno: 2, filename: jade_debug[0].filename });
doStuff();
jade_debug.shift();
jade_debug.unshift({ lineno: 3, filename: jade_debug[0].filename });
}

But in my case:

function foo(bar) {
jade_debug.shift();
jade_debug.unshift({ lineno: 2, filename: jade_debug[0].filename });
doStuff(
jade_debug.shift();
jade_debug.unshift({ lineno: 3, filename: jade_debug[0].filename });
);
jade_debug.shift();
jade_debug.unshift({ lineno: 4, filename: jade_debug[0].filename });
}

But this code gets me dashes on my web page:

- function foo(bar) {
-
-   doStuff();
-
- }

Odd. It throws an error here with Jade 1.9.2:

Jade:2
    1| - function foo(bar) {
  > 2| -
    3| -   doStuff();
    4| -
    5| - }

unexpected text -
-  

This also works:

- function foo(bar) {

-   doStuff();

- }

Again this is another lucky case, and is not supported.

@ivanjuras
Copy link

Is this going to be implemented in the near future?

@ForbesLindesay
Copy link
Member

We need to get better at supporting multi-line JavaScript. We currently only support multi-line if the line break is directly inside a BlockStatement. So what you're attempting to do is generally supported. Not supporting blank lines is just a quirk of the lexer. We can fix that pretty easily. The problem of supporting multi-line expressions more generally as @TimothyGu points out, is much much harder. It would require a complete overhaul of how we deal with errors in jade. What we need to do is move away from inserting debugger statements and instead generate a source map.

@ForbesLindesay
Copy link
Member

The dashes will be treated as block-code as part of the upgrade to [email protected]

@TimothyGu TimothyGu added this to the 2.0.0 milestone Aug 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants