donpark / html2jade

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

Fix breaklines #107

Open smaudet opened 8 years ago

smaudet commented 8 years ago

I would like to submit this for consideration.

Currently, if you use this tool, you have to do a lot of cleanup work to get br to work right. br doesn't expand correctly in jade atm, this proposed fix adds a mixin to the top of jade files with a doctype that replaces calls to br with +bbr(), and inserts a <br /> into the code.

Spacing is odd, however there are already issues in both jade and this project which prevent it from being perfect.

There is a test file, as well as existing tests have been modified to update the expectations.

smaudet commented 8 years ago

I'm trying to make the travis build pass, something broke with your travis.

I think your travis build is hypersensitive to line endings perhaps.

donpark commented 8 years ago

Looks like you've put a lot of work into this PR. But I am going to defer merging it because, at least to me, it seems overly elaborate and opinionated. My being unaware of seriousness of br related issues and shortcomings may have also contributed. Sorry.

Would you like me to link to your branch from the README to help users who share your needs?

smaudet commented 8 years ago

@donpark what part of it seems elaborate and opinionated? I'm fine with an update in your readme to point to this branch.

Seriousness of the <br /> issue is that <br> doesn't always work. It should...but I've come across cases where it didn't, and even according to standard specs, <br /> would work where <br> would not when using non html5, or some form of xhtml. Perhaps there should be a flag that allows toggling this feature?

In making the tests/travis builds work, I noticed somethings that were broken in other areas:

If you're not happy with this feature I might refactor those changes out of here into another branch and PR that at least, so that other users who might share my needs don't have the same problems. Let me know if you think the changes in the areas outlined above merit another separate PR.

Pulling master clean, it also looks like testing is working now after I disabled my global autocrlf = true, so either on windows you may simply need to add a local .gitconfig that turns it off, or put something in the README warning users to change this in their global .gitconfig (which is in general inconvenient for windows users).

donpark commented 8 years ago

Well, the thing is I don't use br in my apps. While the tag is useful sometimes while hacking to try this and that to find a workaround for CSS issues, I've never ran across a use-case where it made good design sense nor the only solution.

FYI, I added link to your branch to README.md.

smaudet commented 8 years ago

Well, that's reasonable I suppose.

I'd like to see this tool being able to do round-trip parsing, i.e. html2jade and jade2html (just using jade), with as close parity as possible.

Of course this is not perfectly possible because of things like mixins/loops, but for simple templates this should be very possible (treat simple mixin like html pages with 'default' parameters, maybe compile some sort of .map file that can hold some information about the loop transformations).

You may be wondering why I think this might be useful - generally I suspect your intended workflow is:

  1. transform legacy html to jade
  2. transform jade to html
  3. never run the html2jade tool again

But if you have someone new to the jade language, it could be very useful to have some sort of approximation like this, or for tools that might do real-time html/jade transformation.

So, from that perspective, any <br> tag that might not work, or other problems using this tool might produce, are issues. <br> was the biggest one which I had seen.