Skip to content

[FEATURE] Represent Selector as Component objects#1496

Merged
oliverklee merged 11 commits into
mainfrom
feature/selector-representation
Feb 9, 2026
Merged

[FEATURE] Represent Selector as Component objects#1496
oliverklee merged 11 commits into
mainfrom
feature/selector-representation

Conversation

@JakeQZ

@JakeQZ JakeQZ commented Feb 5, 2026

Copy link
Copy Markdown
Collaborator

Part of #1325.

@JakeQZ JakeQZ self-assigned this Feb 5, 2026
@JakeQZ JakeQZ added enhancement refactor For PRs that refactor code without changing functionality labels Feb 5, 2026
@JakeQZ JakeQZ marked this pull request as draft February 5, 2026 20:11
@coveralls

coveralls commented Feb 5, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 72.856% (+0.2%) from 72.628%
when pulling 30fcbe1 on feature/selector-representation
into dd1b3de on main.

@JakeQZ JakeQZ force-pushed the feature/selector-representation branch 4 times, most recently from 4a4d0f0 to 0effcdb Compare February 5, 2026 23:44
@JakeQZ JakeQZ marked this pull request as ready for review February 5, 2026 23:55
@JakeQZ JakeQZ requested a review from oliverklee February 5, 2026 23:55
@JakeQZ

JakeQZ commented Feb 5, 2026

Copy link
Copy Markdown
Collaborator Author

Would like initial feedback on this...

There's a bit more to do:

  • A few more tests;
  • Changelog entries.

It might be possible to break this down into seperate PR, but I can't think how - everything is interdependent.

@JakeQZ JakeQZ requested a review from sabberworm February 5, 2026 23:58
@JakeQZ JakeQZ force-pushed the feature/selector-representation branch from 0effcdb to 9927247 Compare February 6, 2026 00:00
@JakeQZ

JakeQZ commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator Author

It might be possible to break this down into seperate PR, but I can't think how - everything is interdependent.

Perhaps an initial change would be to have setSelector() (and the constructor) throw an exception if the selector is invalid, using the existing isValid() method. I wouldn't consider this to be a breaking change, more of a fix, though something that would probably go in the 'Changed' section of the changelog.

@JakeQZ JakeQZ marked this pull request as draft February 6, 2026 01:26
@JakeQZ

JakeQZ commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator Author

Perhaps an initial change would be to have setSelector() (and the constructor) throw an exception if the selector is invalid, using the existing isValid() method. I wouldn't consider this to be a breaking change, more of a fix, though something that would probably go in the 'Changed' section of the changelog.

#1498 covers exceptions possibly being thrown by setSelector() and the constructor.

@JakeQZ JakeQZ force-pushed the feature/selector-representation branch 2 times, most recently from a415794 to a7b1af1 Compare February 6, 2026 09:38
Comment thread src/Property/Selector.php
@JakeQZ JakeQZ force-pushed the feature/selector-representation branch from a7b1af1 to 557149c Compare February 6, 2026 10:44
@JakeQZ

JakeQZ commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator Author

There's a bit more to do:

  • A few more tests;
  • Changelog entries.

Have added the additional tests and changelog entry.

@JakeQZ JakeQZ marked this pull request as ready for review February 6, 2026 10:55
@JakeQZ JakeQZ requested a review from oliverklee February 6, 2026 10:56
@JakeQZ JakeQZ force-pushed the feature/selector-representation branch from 557149c to 2aab41f Compare February 6, 2026 11:48
@JakeQZ

JakeQZ commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator Author

Have added the additional tests and changelog entry.

Now actually committed that update.

JakeQZ added a commit that referenced this pull request Feb 6, 2026
This does not include the string value, because it will be changed in #1496.
@JakeQZ

JakeQZ commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator Author

#1499 partially implements getArrayRepresentation() to reduce the payload of this PR.

@JakeQZ JakeQZ force-pushed the feature/selector-representation branch from 1de46dc to 6f95107 Compare February 9, 2026 18:44
@JakeQZ

JakeQZ commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator Author

I've tidied up a few things, and reduced the diff with a pre-PR. I can't see any other possible pre-PRs, so this is ready for re-review.

@oliverklee oliverklee left a comment

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.

Looks very good! Only one nit.

Comment thread src/Property/Selector.php
@JakeQZ

JakeQZ commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator Author

I spotted a few type annotations in SelectorTest that could be tightened to use non-empty-list or non-empty-string, so have done so, and added a fluent interface test.

@JakeQZ JakeQZ requested a review from oliverklee February 9, 2026 22:32
This now may throw an `UnexpectedTokenException` rather than an
`UnexpectedValueException`.
@JakeQZ

JakeQZ commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator Author

Also now corrected a @throws annotation.

@oliverklee oliverklee left a comment

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.

Nice!

@oliverklee oliverklee merged commit 8e87b8a into main Feb 9, 2026
24 checks passed
@oliverklee oliverklee deleted the feature/selector-representation branch February 9, 2026 23:11
JakeQZ added a commit that referenced this pull request Feb 18, 2026
Don't treat spaces or characters like `~` within them as a stop character
representing a combinator.

The bug was introduced by #1496 and related changes that split selector
representation and parsing into compound selectors and combinators.

Fixes #1533
JakeQZ added a commit that referenced this pull request Feb 18, 2026
Don't treat spaces or characters like `~` within them as a stop character
representing a combinator.

The bug was introduced by #1496 and related changes that split selector
representation and parsing into compound selectors and combinators.

Addresses some of the issues in #1533.
oliverklee pushed a commit that referenced this pull request Feb 18, 2026
Don't treat spaces or characters like `~` within them as a stop character
representing a combinator.

The bug was introduced by #1496 and related changes that split selector
representation and parsing into compound selectors and combinators.

Addresses some of the issues in #1533.
JakeQZ added a commit that referenced this pull request Feb 18, 2026
The bug was introduced by #1496 and related changes that split selector
representation and parsing into compound selectors and combinators.

Addresses some of the issues in #1533.
JakeQZ added a commit that referenced this pull request Feb 18, 2026
The bug was introduced by #1496 and related changes that split selector
representation and parsing into compound selectors and combinators.

Fixes #1533
JakeQZ added a commit that referenced this pull request Feb 18, 2026
The bug was introduced by #1496 and related changes that split selector
representation and parsing into compound selectors and combinators.

The regex for `CompoundSelector::isValid()` now matches that for
`Selector::isValid()`.

Fixes #1533
oliverklee pushed a commit that referenced this pull request Feb 18, 2026
The bug was introduced by #1496 and related changes that split selector
representation and parsing into compound selectors and combinators.

The regex for `CompoundSelector::isValid()` now matches that for
`Selector::isValid()`.

Fixes #1533
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement refactor For PRs that refactor code without changing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants