search

rehypejs / rehype-minify

plugins to minify HTML
https://unifiedjs.com
MIT License
90 stars 16 forks source link

rehype-minify-whitespace: inline element are trimmed #32

Closed kptdobe closed 4 years ago

kptdobe commented 4 years ago

Fixes #19

Inline elements like a, span, em... loses there leading and trailing whitespaces while those might be are important.

This PR is certainly requires some love, happy to improve the code and follow the guidelines.

Note that this changes the current behavior (see existing modified tests).

wooorm commented 4 years ago

Thanks for working on this! The list you added, is already in list.json I believe. This PR is also a bit too much: it keeps too much whitespace in the tree for a project that wants to remove superfluous whitespace.

kptdobe commented 4 years ago

Thanks for working on this! The list you added, is already in list.json I believe.

Not sure how I could have missed that!

This PR is also a bit too much: it keeps too much whitespace in the tree for a project that wants to remove superfluous whitespace.

The only solution I see to make that better is to check the siblings of the parent. But these are not available in the visitor function: the only way you can know if you can trim <strong> some text </strong> is if you know what is before and after the <strong> node, not the text node.

kptdobe commented 4 years ago

I worked a bit more on it, I have added a lot of test cases (and implemented) which are not supported with current version and corrupt the DOM. I reached the point where I realised it could be a pretty complex to support any use case, especially deep nesting of inline elements. Might still fix this too.

wooorm commented 4 years ago

Thanks! I've also been looking into this, sorry I didn't share that before. The tests are very useful!

kptdobe commented 4 years ago

Feel free to copy the tests and write a better / good solution ;)

wooorm commented 4 years ago

Thanks for your help @kptdobe, released!