-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix scanning classes delimited by tab characters #15169
base: next
Are you sure you want to change the base?
Conversation
2a4c256
to
e6b4458
Compare
'test-green', | ||
'test-blue', | ||
'test-tomato', | ||
'multiple-entries-to-keep-newlines', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My prettier hack is I use an empty //
comment to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(but this is fine too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That introduced a space, so didn't trust it 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah!
1a3a8e8
to
fab794a
Compare
fab794a
to
0e5eb84
Compare
Should there be explicit tests for tabs vs. sort of sneaking it into the multi-line arrays test? |
We were only looking at spaces, but we should look at newlines, tabs, ... as well.
0e5eb84
to
48f68b7
Compare
@adamwathan good call, I added dedicated whitespace related tests (none, spaces, tabs, and newlines) in the Rust tests. Also add an integration test for the CLI. |
This PR fixes an issue where multi-line candidates in Svelte files couldn't be found as reported in #15148
After digging in, the real culprit seems to be that the reproduction used tab
\t
characters instead of spaces and we only delimited explicitly on spaces.Initially I couldn't reproduce this in an integration test until we (@thecrypticace and I) realised that
\t
was being used.Test plan:
This PR adds an integration test that fails before the fix happens. The fix itself is easy in the sense that we just use all ascii whitespace characters instead of just spaces.
Fixes: #15148