Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Property/Selector.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Sabberworm\CSS\Renderable;

use function Safe\preg_match;
use function Safe\preg_replace;

/**
* Class representing a single CSS selector. Selectors have to be split by the comma prior to being passed into this
Expand Down Expand Up @@ -74,7 +75,9 @@ public function getSelector(): string

public function setSelector(string $selector): void
{
$this->selector = \trim($selector);
$selector = \trim($selector);

$this->selector = !str_contains($selector, '[') ? preg_replace('/\s{2,}/', ' ', $selector) : $selector;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The \s needs double-escaping (it does technically work single-escaped, but fails a coding style check).

We could also convert any whitespace to an actual space - e.g. a single newline, tab, etc. - and have a test for that too.

And for better performance (and to avoid catastrophic backtracking in the unlikely case of a selector string with a million spaces being passed), we can use the possessive quantifier (+).

Suggested change
$this->selector = !str_contains($selector, '[') ? preg_replace('/\s{2,}/', ' ', $selector) : $selector;
$this->selector = !str_contains($selector, '[') ? preg_replace('/\\s++/', ' ', $selector) : $selector;

}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ public function selectorIgnoresInFile(): void
$document = self::parsedStructureForFile('selector-ignores', Settings::create()->withMultibyteSupport(true));
$expected = '.some[selectors-may=\'contain-a-{\'] {}'
. "\n"
. '.this-selector .valid {width: 100px;}'
. '.this-selector .valid {width: 100px;}'
. "\n"
. '@media only screen and (min-width: 200px) {.test {prop: val;}}';
self::assertSame($expected, $document->render());
Expand Down
25 changes: 25 additions & 0 deletions tests/Unit/Property/SelectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,29 @@ public function isValidForInvalidSelectorReturnsFalse(string $selector): void
{
self::assertFalse(Selector::isValid($selector));
}

/**
* @test
*/
public function cleanupSpacesWithinSelector(): void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public function cleanupSpacesWithinSelector(): void
public function cleansUpSpacesWithinSelector(): void

{
$selector = <<<'CSS'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't use heredoc - indented heredoc doesn't work with with PHP 7.2 (though it works with 7.3 upwards), and it makes autoformatting the code more of a hassle.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead, could you please switch to a literal, quotes-enclosed string?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for bringing this to my attention, it's fixed.

p >
small
CSS;

$subject = new Selector($selector);

self::assertSame('p > small', $subject->getSelector());
}

/**
* @test
*/
public function doNotCleanupSpacesWithinAttributeSelector(): void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public function doNotCleanupSpacesWithinAttributeSelector(): void
public function doesNotCleanupSpacesWithinAttributeSelector(): void

{
$subject = new Selector('a[title="extra space"]');

self::assertSame('a[title="extra space"]', $subject->getSelector());
}
}
Loading