Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

kptdobe
Copy link
Contributor

@kptdobe kptdobe commented May 28, 2020

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
Copy link
Member

wooorm commented May 29, 2020

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
Copy link
Contributor Author

kptdobe commented Jun 2, 2020

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
Copy link
Contributor Author

kptdobe commented Jun 3, 2020

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
Copy link
Member

wooorm commented Jun 3, 2020

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

@kptdobe
Copy link
Contributor Author

kptdobe commented Jun 3, 2020

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

@wooorm wooorm closed this in 64c6206 Jun 4, 2020
@wooorm
Copy link
Member

wooorm commented Jun 4, 2020

Thanks for your help @kptdobe, released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

rehype-minify-whitespace: inline HTML content is trimmed
2 participants