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

Broken highlighting in Build 3154 #107

Closed
n-kort opened this issue Nov 12, 2017 · 18 comments
Closed

Broken highlighting in Build 3154 #107

n-kort opened this issue Nov 12, 2017 · 18 comments

Comments

@n-kort
Copy link

n-kort commented Nov 12, 2017

screen shot 2017-11-12 at 12 09 17

Also posted over at sublimehq/Packages#1302

@Thom1729
Copy link
Contributor

The way that I would fix this is by using the new embed feature, like the core HTML syntax does. This would require updating the syntax definition to the sublime-syntax format, which would break compatibility with ST2. I'm not familiar enough with the tmLanguage format to say whether there is a solution within that system.

@yyx990803
Copy link
Member

I'd love to rewrite this using the sublime-syntax format, but currently don't have time to do so myself. If anyone wants to take a stab at it that would be awesome..

@skyronic
Copy link
Collaborator

skyronic commented Nov 14, 2017

There's a temporary workaround to fix this with the current tmLanguage file: Install the Babel extension and use package manager to disable the JavaScript package (important to disable JS package). This seems to fix it for me. Can you try this @n-kort

image

@n-kort
Copy link
Author

n-kort commented Nov 14, 2017

@skyronic doesn't quite work for me:

screen shot 2017-11-14 at 10 31 32

Just loses all highlighting in the <script/> block. This was after installing the Babel package and adding "JavaScript" to "ignored_packages"...

@skyronic
Copy link
Collaborator

@n-kort can you have your cursor somewhere in the script block and press control+shift+p (OSX) or ctrl+alt+shift+p (Windows and Linux).

It should pop up some text. can you paste that here?

@n-kort
Copy link
Author

n-kort commented Nov 14, 2017

text.html.vue
source.js.embedded.html

with the cursor just after 'bar'

@skyronic
Copy link
Collaborator

Ok. I'll send a early version of the syntax definition sometime within 12 hours, hopefully you can try that and tell me if it works for you :)

@skyronic
Copy link
Collaborator

@n-kort can you try the instructions from the PR at #109 and tell me if that fixes your issue?

@skyronic
Copy link
Collaborator

@n-kort @yyx990803 I can confirm that in the latest dev build of sublime, it seems to be working when I disable JavaScript and install Babel. In fact, I'm thinking the issue is actually with sublime's JS implementation.

Please see https://gfycat.com/anotherseriousfritillarybutterfly for a quick screencast.

@skyronic
Copy link
Collaborator

Filed an issue with Sublime for their JS syntax implementation which looks wrong sublimehq/Packages#1309

@Thom1729 if possible, can you verify if my issue is valid and is something that needs to be fixed from Sublime's end?

@n-kort
Copy link
Author

n-kort commented Nov 15, 2017

@skyronic yep that's what I'm getting too

@Thom1729
Copy link
Contributor

Sublime's JavaScript package has changed. The core JavaScript package's old behavior and the Babel package's behavior are incorrect. The core JavaScript package's new behavior is correct.

Here's another example:

var x, g;
export default {
    foo: 42
}
/x/g;

The Babel package will erroneously mark /x/g as a regular expression literal. The new JavaScript syntax will correctly mark it as an arithmetic expression. If you try it on Babel's online REPL, you will see that this is how Babel treats it as well.

@skyronic
Copy link
Collaborator

skyronic commented Nov 15, 2017

(comment removed since it was irrelavant and you'd answered the question elsewhere)

@Thom1729
Copy link
Contributor

On the contrary, that is exactly what should happen. Here are near-minimal examples:

export default 2 + 2;
export default 2; + 2;

The first line exports the value of the expression 2 + 2. Adding a semicolon changes the meaning of the second line; now, you have a export statement export default 2; and an expression statement + 2;. In the first line, the + is the binary addition operator, and in the second line it is the unary number coercion operator. The only difference is the semicolon, but the two lines have different interpretations.

Now imagine that there is a line break after the first 2 on each line:

export default 2
+ 2;
export default 2;
+ 2;

The meanings of these statements are not changed by the line break. There are only a few cases in JavaScript where adding a line break will change the meaning of a valid statement, and this is not one of them. Suppose that you are a syntax highlighter and you see:

export default 2

// This space intentionally left blank

You can't assume that the statement will end at the first line break. You can't assume that the statement has ended until you encounter a semicolon (or a line break followed by a token that triggers automatic semicolon insertion). You're still inside an export statement until something kicks you out of it, no matter how long it may take.

The JavaScript syntax can be very unintuitive at times, particularly where missing semicolons are concerned. I would bet good money that the new implementation isn't perfect. But I am very confident that its behavior in this case is correct and that the previous behavior was a bug.

Notwithstanding any of the above, even if everything I've said is wrong and the old behavior was correct, the way that the Vue syntax embeds JavaScript relies implicitly upon a false assumption about the core JavaScript syntax: that when the JavaScript code ends, only the main context will be on the stack. The new JavaScript syntax violates this assumption, resulting in this bug. But it already violated that assumption in other cases, resulting in #101. Fixing JavaScript embedding to use with_prototype or (preferably) embed/escape will resolve both of these bugs and many more.

@alancwoo
Copy link

@n-kort @skyronic I can also confirm that disabling the default javascript package and using babel works. However, I personally had to remove and reinstall the babel package for it to work properly. I'm on ST dev channel build 3154

@Thom1729
Copy link
Contributor

@alancwoo This is due to bugs in babel-sublime that have been fixed in the core JavaScript package. If those bugs are ever fixed, the Vue syntax will break when using babel-sublime.

@alancwoo
Copy link

Note: Reverting sublime to the stable channel with the core JavaScript package also fixes the issues (Build 3143)

@skyronic
Copy link
Collaborator

Hey @n-kort you should be able to install the latest version from package control and should fix the issue. Lemme know if you encounter any other problems with the new highlighting! Closing this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants