Skip to content

Conversation

@xHeaven
Copy link
Contributor

@xHeaven xHeaven commented Jan 8, 2026

Closes #1867.

Copy link
Contributor

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite correct. Whitespace introduced by linebreaks also needs to be preserved. See: https://jsfiddle.net/rh7y1xg4/

@innocenzi innocenzi requested a review from brendt January 8, 2026 12:20
@xHeaven
Copy link
Contributor Author

xHeaven commented Jan 8, 2026

@TimWolla Can you take a look now please?

Copy link
Contributor

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test assertions LGTM. Cannot comment on the actual logic changes, due to my lack of familiarity with them.

I'd like to note one more thing, though: Elements with white-space: pre CSS (or a corresponding class) will still be affected either way. Technically removing whitespace is never safe (at least not within the body).

@xHeaven
Copy link
Contributor Author

xHeaven commented Jan 8, 2026

I'd like to note one more thing, though: Elements with white-space: pre CSS (or a corresponding class) will still be affected either way. Technically removing whitespace is never safe (at least not within the body).

@brendt I think I might need some help with this 👀

@brendt
Copy link
Member

brendt commented Jan 9, 2026

I thought about it this night and actually think I want to take another approach: we can add a new whitespace token to the lexer and simply keep whitespaces as is, always. It also means we wouldn't have to introduce another regex match, which is always good.

I'm not sure if this would have any other implications, so I'm going to fiddle around with it (or you can @xHeaven if you want to).

Anyway I'll close this PR but I'll copy over the tests

@brendt brendt closed this Jan 9, 2026
@xHeaven xHeaven deleted the whitespace branch January 9, 2026 16:57
@xHeaven
Copy link
Contributor Author

xHeaven commented Jan 9, 2026

I thought about it this night and actually think I want to take another approach: we can add a new whitespace token to the lexer and simply keep whitespaces as is, always. It also means we wouldn't have to introduce another regex match, which is always good.

I'm not sure if this would have any other implications, so I'm going to fiddle around with it (or you can @xHeaven if you want to).

Anyway I'll close this PR but I'll copy over the tests

Your solution turned out way more elegant after all. Good work!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Views incorrectly removes meaningful whitespace between HTML elements

3 participants