search

subtleGradient / language-javascript-jsx

JavaScript with JSX Bundle for Atom
45 stars 6 forks source link

Locks up atom when attempting to type closing </p> #4

Closed jhopper28 closed 9 years ago

jhopper28 commented 9 years ago

To reproduce:

  1. At the beginning of an existing line with a JSX component (or on a blank line), type <p>
  2. Go to end of line and type < Atom locks up.
subtleGradient commented 9 years ago

Thanks for the bug report! I'll look into this when I can

subtleGradient commented 9 years ago

Confirmed. This happens for me too

subtleGradient commented 9 years ago

Oh crap! This may be my fault in a MUCH deeper way than I could have guessed!

Looks like this may be a problem in atom/slick, which is a fork of kamicane/slick, which is a project created by @kamicane, @subtlegradient (that's me), and @fabiomcosta!

Awesome!

fabiomcosta commented 9 years ago

Wait, Atom uses Slick? That's new to me, cool!

subtleGradient commented 9 years ago

@kevinsawicki, do you think this could be a Slick issue? I haven't touched that code in a LONG time. I'll need to set aside some time to look into it properly.

One thing I know is that the Slick parser was never built to handle TextMate 2 selectors. In this language grammar, I'm using a bunch of replacement strings. e.g. foo.bar.baz.${2/something/}. Slick certainly can't handle that stuff, but I wouldn't have thought it would ∞ loop because of it. Clearly I didn't do enough testing :P

subtleGradient commented 9 years ago

Cf. https://github.com/subtleGradient/language-javascript-jsx/blob/master/grammars/JavaScript%20with%20JSX.cson#L121

subtleGradient commented 9 years ago

Actually, it looks like Slick is properly parsing meta.block.inner.${2/[:.]/-/g}.jsx as a meta element with the classList ['meta', 'block', 'inner', '${2/[:', ']/-/g}', 'jsx']. Lulz

subtleGradient commented 9 years ago

@Inviz, do you have an improved version of the Slick parser hanging out anywhere?

kevinsawicki commented 9 years ago

Yeah, I've seen slick lock up in the past which is why Atom ships with fork of it, may need some tweaking to prevent other lockup scenarios

apostoiis commented 9 years ago

Yeap, this happens to me too. I'm writing the closing tag element first, and then I add the / so Atom doesn't crash. )

subtleGradient commented 9 years ago

Just published v0.3.3 with a fix for this. Slick may have been a red herring.

Pro Tip™ TextMate handles way more edge cases than Atom. Scopes that select zero characters seem to be a problem.

apostoiis commented 9 years ago

Just tested it, works fine. thanks :)

subtleGradient commented 9 years ago

My fix in v0.3.3 broke JSX close tags without newlines before them. I just released v0.3.4 that fixes that new bug without unfixing this issue also. But I also added some extra scope to hi light improper close tags as invalid. Everything should be working great, but if not, as always, please do let me know.

Thanks much!