donpark / html2jade

Converts HTML to Jade template. Not perfect but useful enough for non-daily conversions.
MIT License
1.18k stars 157 forks source link

Current version adds unnecessary empty | lines #93

Closed DomiR closed 9 years ago

DomiR commented 9 years ago

The newest version is adding unnecessary empty lines everywhere. I don't think this is supposed to do so and was introduced by the new version.

From the online tool: http://html2jade.aaron-powell.com/ using 0.8.1

<div class="two fields">
    <div class="field">
      <label>First Name</label>
      <input placeholder="First Name" type="text">
    </div>
 </div>
.two.fields
    .field
        label First Name
        |         <- This should not be here!!!
        input(placeholder='First Name', type='text')

Kind regards

donpark commented 9 years ago

This is a side-effect of recent fix to preserve white-spaces. I'll get to this eventually but feel free to submit a PR.

donpark commented 9 years ago

I don't have time to fully review issues involved in whitespace handling but how do you propose html2jade determine when whitespace is necessary and when it's not without adding HTML DTD awareness?

pmeissner commented 9 years ago

After it's done processing, could it loop through the resulting code looking for lines that contain a single pipe character only and eliminate those lines?

donpark commented 9 years ago

I think that will remove some intentionally added white-spaces. White-spaces can be ignored, compressed to a single space, or must be left as is. DTD helps differentiate but doesn't cover ugly corner cases.

AFAIK, I think whitespace between label end-tag and input start-tag in the example above is same as a single space when rendered so it can't be removed.

Nedomas commented 9 years ago

This is a huge issue, because a lot of editors have auto trailing space trimming.

I already get tons of errors like these:

Message:
    /Users/domas/Developer/test-app1/index.jade:4
    2|   head
    3|     title Beautiful Jade
  > 4|     |
    5|     link(rel='stylesheet', href='vendor/css/bootstrap.min.css')
    6|     |
    7|     link(rel='stylesheet', href='css/app.css')

unexpected text |
donpark commented 9 years ago

When I have time (warning: not for a while), I'll revisit this issue. One possible compromise is to add an optional flag that filters out white-space only piped lines.

blunckr commented 9 years ago

If anyone is looking for a quick fix for this, I just did a find/replace in sublime text with this regex: /^\s+\|\s+$\n/

It removes any line that only has a pipe character. Of course It might remove lines that you actually want, you'll have to just find those manually.

ndastur commented 9 years ago

This a major issue, I think. My suggestion would be to roll back the commit until time becomes available to look into a solution. +1 for agrees ;)

donpark commented 9 years ago

OK. bumped to major status.

When I find some free time, I'll add a compiler flag to wipe out all 'lone pipe' lines.

Pull request could quicken the process however.