creationix / haml-js

Haml ported to server-side Javascript. This is a traditional server-side templating language. Tested with node-js
MIT License
902 stars 110 forks source link

Attribute parsing fails #81

Open stmichael opened 10 years ago

stmichael commented 10 years ago

I'm trying to translate a HAML template to HTML. The template looks as like this:

.a-class{"data-abc" => "my value"}

According to the HAML reference I expect it to be translated to:

<div class="a-class" data-abc="my value"/>

But haml-js always ignored the data-abc part. I couldn't figure out why until I dug deeper into the code and discovered that my HAML syntax was wrong. It should have been:

.a-class{"data-abc": "my value"}

(notice the : instead of =>)

Why does haml-js ignore the =>-Syntax? The reference clearly states that it is part of HAML.

ablohowiak-fanhattan commented 10 years ago

I encourage you to send a pull request with support for => syntax and tests.

stmichael commented 10 years ago

I'm kind of undecided at how to address this problem. Could you give me some advice on this?

My changes will affect the function parse_attribs. At first I thought of looking for the join operators : and => instead of just :. But then I realized that this could get tricky since the main loop to parse the attributes is character based. Since => is longer than one character the loop would have to have some kind of memory of the parts of the join operator that have already been parsed.

Another idea is to replace the => syntax with the : syntax using regex before the parsing starts.

What do you think about these two approaches?

ablohowiak-fanhattan commented 10 years ago

Using regex is very tricky -- is it valid to have '=>' in an attribute? Hmm, I don't think so, but would have to check. If we are sure that '=>' is invalid as an attribute, then the regex solution might be the clearest path forward.

The single-character no-lookahead nature of the attrib parser is one of the reasons that I didn't implement this in the past =/ a way to peak and auto-advance would be welcome, but more work.

stmichael commented 10 years ago

=> in an attribute name doesn't make much sense since it would break the HTML structure completely. But => in the value part is possible. In angularjs for instance you can use expressions that contain comparisons with standard operators like this one.

ablohowiak-fanhattan commented 10 years ago

So the regex way seems like it isn't viable, then :(

roscom commented 10 years ago

Further to the above:

      %a{ng_href: "#"}  a message //1
      %a{data: {ng_href: "#"}}  a message //2
      %a{data: {ng-href: "#"}}  a message //3
      %a{'ng-href': ""}  a message //4

All of the above syntaxes fail to compile and cause the compiler to barf!. All but number 3 are valid haml syntax.

The only way to write the above such that it compiles to the desired output is

 %a{ng-href: "#"}  a message
- or -
%a{data-ng-href: "#"}  a message

even though it is invalid syntax.

Any chance this could be fixed soon? Be a very big help.

Kind regards Ross

PS: Haml-JS is used in grunt-haml

I'd also recommend the use of : in place of => as => has been deprecated in ruby 1.9+

stmichael commented 10 years ago

Since the solution seems to be quite time expensive (writing a look ahead parser), I cannot do it in the scope of our project. Sorry. :cry:

Googling for a possible improvement for the parser, I found http://pegjs.majda.cz/. You define a language grammar (similar to the Backus-Naur form) and the library will generate a parser library in javascript. The HAML source will be the input for the parser, the output will be in the form you specified in the language grammar. This means that we would already have a tokenizer. The parser would have to be rewritten to interpret the output of the tokenizer which is significantly simpler to do than to parse the HAML source directly. Also adding a feature like the one described here would only be a change in the language grammar :smile: @ablohowiak-fanhattan what do you think of such a change?

I implemented a simple work around for my problem. I replace all occurrences of the => syntax with the : syntax. Fortunately we only use haml-js with my ugly hack in the test environment.

StoneCypher commented 9 years ago

This is concerning. I am curious whether any progress has been made here.