Skip to content

[TASK] Allow construction of 'empty' Selector#1500

Closed
JakeQZ wants to merge 2 commits into
mainfrom
task/blank-selector
Closed

[TASK] Allow construction of 'empty' Selector#1500
JakeQZ wants to merge 2 commits into
mainfrom
task/blank-selector

Conversation

@JakeQZ

@JakeQZ JakeQZ commented Feb 7, 2026

Copy link
Copy Markdown
Collaborator

This was already allowed. This change merely clarifies the behaviour, and makes the constructor parameter optional.

The parse() method is static, so it is more convenient if it can create an empty object, then set the value via setter methods.

This was already allowed.  This change merely clarifies the behaviour, and
makes the constructor parameter optional.

The `parse()` method is static, so it is more convenient if it can create an
empty object, then set the value via setter methods.
@JakeQZ JakeQZ requested a review from oliverklee February 7, 2026 00:25
@JakeQZ JakeQZ self-assigned this Feb 7, 2026
@JakeQZ JakeQZ added cleanup refactor For PRs that refactor code without changing functionality labels Feb 7, 2026
@coveralls

coveralls commented Feb 7, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 72.641% (+0.01%) from 72.628%
when pulling 2c62814 on task/blank-selector
into e4c7f2c on main.

Comment thread src/Property/Selector.php
* @throws \UnexpectedValueException if the selector is not valid
*/
final public function __construct(string $selector)
final public function __construct(string $selector = '')

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 cover creating a selector (and thus having an empty string for a selector) without passing the $selector with a test.

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.

That's already been done, in #1498.

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.

I don't see a test that uses new Selector() in #1498 - but I'm still at my first coffee. Am I missing something?

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.

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've added an additional test for construction without a parameter.

@JakeQZ JakeQZ requested a review from oliverklee February 8, 2026 06:17
@JakeQZ JakeQZ force-pushed the task/blank-selector branch from b61bd50 to 2c62814 Compare February 8, 2026 06:24
@JakeQZ

JakeQZ commented Feb 8, 2026

Copy link
Copy Markdown
Collaborator Author

As per #1496 (comment), this isn't needed.

@JakeQZ JakeQZ closed this Feb 8, 2026
@JakeQZ JakeQZ deleted the task/blank-selector branch February 8, 2026 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup refactor For PRs that refactor code without changing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants