mbylstra / html-to-elm

An online tool for converting HTML to elm-html. Go to
http://mbylstra.github.io/html-to-elm/
395 stars 23 forks source link

Does not parse single quote values #8

Closed victorlcampos closed 8 years ago

victorlcampos commented 8 years ago

image

mbylstra commented 8 years ago

It looks like the issue is actually with the parser failing to parse single quotes (not to do with the class property). This example will work if you use double quotes for the value of class:

<body>  
  <div class="hello">
    <h1>Hello world</h1>
  </div>
</body>

output:

body []
    [ div [ class "hello" ]
        [ h1 []
            [ text "Hello world" ]
        ]
    ]

I've changed the issue title to reflect this.

victorlcampos commented 8 years ago

Hi, thx. I will take a look on this problem and send pr

mbylstra commented 8 years ago

No problem. The HTML parsing code is a bit hairy I'm afraid. It was first time I've written a parser, so it was a bit of a learning curve as I was learning Elm at the same time. It's not very well documented. One day I hope to rewrite it so that it can be used more generally, or just use a better parser library that someone else has written. Let me know if you have any questions. The failure to parse single quotes was just laziness on my behalf, as single quotes aren't used so often in HTML.

mbylstra commented 8 years ago

If you do have a go at it, the line you'll need to look at is at https://github.com/mbylstra/html-to-elm/blob/master/elm-src/HtmlParser/HtmlParserRawAst.elm#L56.

victorlcampos commented 8 years ago

Yes, it's my first time with elm. How can I run this? =|

mbylstra commented 8 years ago

If you clone the repo, you can do:

elm reactor

then go to http://localhost:8000/website/index.html in your browser.

You can compile using gulp elm or gulp watch (you need gulp installed and you need to run npm install first from within the repo directory).

victorlcampos commented 8 years ago

Sry @mbylstra, I have to go out now, I'm try to understand, but not successfully. If you fix this, plx mark me to see how.

mbylstra commented 8 years ago

Fixed. If you look at https://github.com/mbylstra/html-to-elm/commit/8f3e0727ce83225fdc4b501f7cd63fecb306937b, the fix should be reasonably self-exaplanatory. (Ignore the code in elm.js and website/js/Main.js - that's just the compiled output).

The function

createParseAnyFunction
         [ parseDoubleQuotationMarkIgnore
         , parseSingleQuotationMarkIgnore
         ]

will parse either a single quote or a double quote, and discard it.

Well, the fix isn't completely correct. It will permit "class' (a mixture of single quotes and double quotes), but it's an improvement.

victorlcampos commented 8 years ago

I didn't understand why (DoubleQuotationMark, "\"|\'") wasn't enough

mbylstra commented 8 years ago

Yes, that should also work.

victorlcampos commented 8 years ago

I tried this, but i got an error... Late i print it here

victorlcampos commented 8 years ago

image

mbylstra commented 8 years ago

I could get the regex to work with "(\"|\')", but I'm not sure why the brackets in the regex are necessary.

victorlcampos commented 8 years ago

Thx, u are doing a great job 😊