Skip to content

[BUGFIX] Allow commas in attributes in setSelectors - #1419

Merged
oliverklee merged 2 commits into
mainfrom
bugfix/setselectors
Dec 9, 2025
Merged

[BUGFIX] Allow commas in attributes in setSelectors#1419
oliverklee merged 2 commits into
mainfrom
bugfix/setselectors

Conversation

@JakeQZ

@JakeQZ JakeQZ commented Dec 9, 2025

Copy link
Copy Markdown
Collaborator

This fixes an issue noted in #1324.

@JakeQZ
JakeQZ requested a review from oliverklee December 9, 2025 14:26
@JakeQZ JakeQZ self-assigned this Dec 9, 2025
@JakeQZ JakeQZ added the bug label Dec 9, 2025
@coveralls

coveralls commented Dec 9, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 63.014% (+0.2%) from 62.804%
when pulling 268f57c on bugfix/setselectors
into a5d1172 on main.

Comment thread src/RuleSet/DeclarationBlock.php Outdated
*/
public function setSelectorsThrowsExceptionWithInvalidSelector(string $selector): void
{
$this->expectException(UnexpectedTokenException::class);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should also check for the exception message.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I looked at the messages and found they were unhelpful, so have fixed that.

I also found that the exceptions thrown by the code when { or end of string were not as expected were not being tested, so have added more test data for that.

Also make it more useful.

And add additional test data to fully exercise the code.
@JakeQZ
JakeQZ requested a review from oliverklee December 9, 2025 18:32
@oliverklee
oliverklee merged commit 8d123a2 into main Dec 9, 2025
23 checks passed
@oliverklee
oliverklee deleted the bugfix/setselectors branch December 9, 2025 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants