HarlemSquirrel / language-haml

Haml language grammar for GitHub's Atom IDE
MIT License
33 stars 24 forks source link

Interpolation with question mark without quotes in :javascript filter screws up highlighting #72

Closed flvrone closed 6 years ago

flvrone commented 7 years ago

Here's the screenshot screenshot from 2017-09-04 13 55 17

HarlemSquirrel commented 7 years ago

It seems like this may actually not be valid Haml

➤  haml -c sample.html.haml
sample.html.haml:2: syntax error, unexpected tSTRING_DEND
:javascript
  var booleanVar = #{@boolean_method?};
.content
  .row.nopadding
    .col-xs-12.text-center
      Something

Even the syntax highlighting here on Github does not like it. If you add quotes, it seems to work.

:javascript
  var booleanVar = "#{@boolean_method?}";
.content
  .row.nopadding
    .col-xs-12.text-center
      Something
➤  haml -c sample.html.haml
Syntax OK
darrenterhune commented 7 years ago

I think this would be a question for what ever syntax theme you've got installed, not this language package. But as @HarlemSquirrel has mentioned, that haml doesn't seem like it's valid.

flvrone commented 7 years ago

@HarlemSquirrel That is very strange, as that code actually works perfectly, and there is almost same example (but method name doesn't include question mark) in the official documentation, here: http://haml.info/docs/yardoc/file.REFERENCE.html#ruby-interpolation- (scroll down a bit for example with filters) And this resource: https://haml2erb.org/ is converting that code using herbalizer, though it doesn't convert interpolations correctly... :(

@darrenterhune I'm using a default white theme, I don't think it could be broken...

Anyway, I will try to make some research later, who knows, maybe the bug is actually inside HAML itself.

HarlemSquirrel commented 7 years ago

@FunkyloverOne It does seem like you've identified an inconsistency within Haml itself. It seems to me that it should be valid given https://stackoverflow.com/questions/10542354/what-are-the-restrictions-for-method-names-in-ruby

But,haml-lint does seem to be aligned with the haml parser on this one.

ezekg commented 6 years ago

Maybe it's because an instance variable (or any variable) can't include ? in Ruby, only method names can include ?. Your @boolean_method? is actually a variable, not a method (seeing as it starts with @, which suggests an instance variable).

class Example
  instance_variable_set '@bool?', 'foobar'
end
# > `@bool?' is not allowed as an instance variable name

Your Haml above doesn't compile for me when using Rails. Yet doing something like this:

class Foo
  def boolean_method?
    true
  end
end

class ApplicationController < ActionController::Base
  protect_from_forgery with: :exception

  def index
    @foo = Foo.new
  end
end
:javascript
  var booleanVar = #{@foo.boolean_method?};
.content
  .row.nopadding
    .col-xs-12.text-center
      Something

works fine and doesn't break syntax highlighting (well, it does break on GitHub, but I don't think they're using this Haml package anymore). So I would assume it's because the Ruby parser is breaking because @boolean_method? is not valid Ruby.

flvrone commented 6 years ago

@ezekg good catch, I've made a mistake in that example, excuse me, here's the correct one: screenshot from 2017-11-16 14 09 53

And here's same with quotes, which is OK: screenshot from 2017-11-16 14 14 21

And here's the code, so you can easily copy it:

:javascript
  var booleanVar = #{boolean_method?};
.class-with-number-6-here
  .class-with-number-12.another-class
    -# commented out code
    -# - if @bool_variable
    -#   .col-xs-12
    -#     number with unit 12kg
    text after quote 'lorem ipsum
    a bit more of text
  .some-class
    = form_for @user do |f|
      .label Email
      = f.email_field :email
      - if @bool_variable
        ...

BTW, it breaks on GitHub in exactly same way. Here's quoted version for consistence:

:javascript
  var booleanVar = "#{boolean_method?}";
.class-with-number-6-here
  .class-with-number-12.another-class
    -# commented out code
    -# - if @bool_variable
    -#   .col-xs-12
    -#     number with unit 12kg
    text after quote 'lorem ipsum
    a bit more of text
  .some-class
    = form_for @user do |f|
      .label Email
      = f.email_field :email
      - if @bool_variable
        ...
duien commented 6 years ago

I just ran into this same issue -- if I have a question-mark inside a non-quoted interpolation syntax highlighting is broken for the rest of the file. Looking at the CSS classes Atom is generating, it appears that the overall grammar doesn't reset back to Haml at the end of the JS block, but instead continues to try to syntax highlight the rest of the file as if it were JS as well.

duien commented 6 years ago

Looking into this a bit more, I suspect that the root of the issue is that when including the source.js grammar for that section, the JS grammar has no concept of Ruby's interpolation syntax. Therefore, the question mark is getting highlighted as a ternary operator and then the fact that there's no following : in its scope messes up the rest of the grammar. You can (to some extent) verify this by adding a : inside the interpolation. Even though that's obviously not valid Ruby it fixes the highlighting (both in Atom and here on Github)

:javascript
  var booleanVar = #{boolean_method?:}; // <-- note the :
.class-with-number-6-here
  .class-with-number-12.another-class
    -# commented out code
    -# - if @bool_variable
    -#   .col-xs-12
    -#     number with unit 12kg
    text after quote 'lorem ipsum
    a bit more of text
  .some-class
    = form_for @user do |f|
      .label Email
      = f.email_field :email
      - if @bool_variable
        ...

I'm not familiar enough with creating grammars to understand how to fix this, especially since it's within a second included grammar, but I'd be happy to give it a shot if I can track down some sufficiently in-depth documentation.

darrenterhune commented 6 years ago

Could just do something like this instead:

.some-div{data: { something: boolean_method? }}

:javascript
  var booleanVar = document.querySelector('.some-div').getAttribute('data-something');

.class-with-number-6-here
  .class-with-number-12.another-class
    -# commented out code
    -# - if @bool_variable
    -#   .col-xs-12
    -#     number with unit 12kg
    text after quote 'lorem ipsum
    a bit more of text
  .some-class
    = form_for @user do |f|
      .label Email
      = f.email_field :email
      - if @bool_variable
        ...

When I interact between ruby and javascript I usually always use data-attributes for this type of thing.

HarlemSquirrel commented 6 years ago

I am still concerned about changing our syntax highlighting when the Haml parser itself says this is invalid. I just tried these examples again with Haml 5.0.4 with the same results.

Has anyone attempted to raise an issue at the https://github.com/haml/haml repo?

HarlemSquirrel commented 6 years ago

This appears to not be valid Haml still as of Haml 5.0.4

➤  cat sample.html.haml
:javascript
  var booleanVar = #{@boolean_method?};
.content
  .row.nopadding
    .col-xs-12.text-center
      Something
➤  haml -c sample.html.haml 
sample.html.haml:2: syntax error, unexpected tSTRING_DEND
Exception on line 307: exit
  Use --trace for backtrace.
➤  haml sample.html.haml                                                                                                              1 ↵
Syntax error on line 128: sample.html.haml:2: syntax error, unexpected tSTRING_DEND
...ooleanVar = #{@boolean_method?};\n", _hamlout.options))
...                              ^
sample.html.haml:3: syntax error, unexpected $undefined, expecting tSTRING_DEND
....to_s);; _hamlout.buffer << ("\n<div class='content'>\n<div ...
...                              ^
sample.html.haml:3: syntax error, unexpected keyword_class, expecting keyword_do or '{' or '('
...amlout.buffer << ("\n<div class='content'>\n<div class='row ...
...                          ^~~~~
sample.html.haml:3: syntax error, unexpected $undefined
...r << ("\n<div class='content'>\n<div class='row nopadding'>\...
...                              ^
sample.html.haml:3: syntax error, unexpected keyword_class, expecting keyword_do or '{' or '('
...iv class='content'>\n<div class='row nopadding'>\n<div class...
...                          ^~~~~
sample.html.haml:3: syntax error, unexpected $undefined
...>\n<div class='row nopadding'>\n<div class='col-xs-12 text-c...
...                              ^
sample.html.haml:3: syntax error, unexpected keyword_class, expecting keyword_do or '{' or '('
...ss='row nopadding'>\n<div class='col-xs-12 text-center'>\nSo...
...                          ^~~~~
sample.html.haml:3: syntax error, unexpected $undefined
...class='col-xs-12 text-center'>\nSomething\n</div>\n</div>\n<...
...                              ^
sample.html.haml:3: unknown regexp options - dv
sample.html.haml:3: syntax error, unexpected $undefined
...r'>\nSomething\n</div>\n</div>\n</div>\n".freeze);; 
...                              ^
sample.html.haml:5: unterminated regexp meets end of file
;_erbout
        ^
sample.html.haml:5: unmatched close parenthesis: /div>\n".freeze);; 
; 
;_erbout/m
sample.html.haml:5: syntax error, unexpected end-of-input, expecting tSTRING_DEND
;_erbout
        ^
  Use --trace for backtrace.

I don't think we need to address this here until it's valid in Haml.