Skip to content

Conversation

thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented Sep 1, 2025

Fixes #328
Fixes #337
Fixes #305

Copy link
Member

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

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

Looks good, just a question though:

tests/tests.ts Outdated
[
`<div [ngClass]="' flex ' + ' underline ' + ' block '"></div>`,
`<div [ngClass]="'flex ' + ' underline' + ' block'"></div>`,
`<div [ngClass]="'flex ' + ' underline ' + ' block'"></div>`,
Copy link
Member

Choose a reason for hiding this comment

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

Hah, oops

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can add an additional test with some whitespace in the middle.

[
  `<div [ngClass]="' flex ' + ' italic      underline ' + ' block '"></div>`,
  `<div [ngClass]="' flex ' + ' italic underline ' + ' block '"></div>`,
]

Just to ensure that we are still taking care of whitespace in the middle where it's safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically there's a lot more cases where it's safe to drop these but that requires a more complex "understanding" of the AST.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the test 👍

@thecrypticace thecrypticace merged commit 30f8d8e into main Sep 2, 2025
1 check passed
@thecrypticace thecrypticace deleted the fix/aggressive-whitespace-removal branch September 2, 2025 14:34
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.

over-aggressive whitespace removal over-aggressive whitespace removal Whitepace still removed incorrectly for some ternary conditions
2 participants