Skip to content

Refactor code in CSSList #994

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.

Sign up for GitHub

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

Jump to bottom
Closed
10 tasks done
oliverklee opened this issue Feb 25, 2025 · 0 comments · Fixed by #1245
Closed
10 tasks done

Refactor code in CSSList #994

oliverklee opened this issue Feb 25, 2025 · 0 comments · Fixed by #1245
Assignees
@oliverklee
Milestone
8.9.0: Deprecatio...

Comments

@oliverklee
Copy link
Collaborator

oliverklee commented Feb 25, 2025 •
edited by JakeQZ
Loading

The goal is to get rid of redundant code and to reduce functions that have return parameters (which are both hard to understand as well as hard analyze with PHPStan).

  • move getAllDeclarationBlocks from Document to the parent class CSSBlockList
  • refactor getAllDeclarationBlocks
  • replace usages of CSSBlockList::allDeclarationBlocks with calls to ::getAllDeclarationBlocks and drop allDeclarationBlocks
  • move getAllRuleSets from Document to the parent class CSSBlockList
  • refactor getAllRuleSets
  • replace usages of CSSBlockList::allRuleSets with calls to ::getAllRuleSets and drop allRuleSets
  • move getAllValues from Document to the parent class CSSBlockList
  • refactor getAllValues
  • replace usages of CSSBlockList::allValues with calls to ::getAllValues and drop allValues
  • rename CSSBlockList::allSelectors to ::getAllSelectors and change its interface to return the result
@oliverklee oliverklee self-assigned this Feb 25, 2025
oliverklee added a commit that referenced this issue Feb 25, 2025
@oliverklee
[CLEANUP] Refactor Document::getAllDeclarationBlocks()
fec6092 
This is a pre-patch to moving `getAllDeclarationBlocks()` up
to `CSSBlockList`.

Part of #994.
@oliverklee oliverklee mentioned this issue Feb 25, 2025
Merged
oliverklee added a commit that referenced this issue Feb 25, 2025
@oliverklee
[CLEANUP] Refactor Document::getAllDeclarationBlocks()
9384d39 
This is a pre-patch to moving `getAllDeclarationBlocks()` up
to `CSSBlockList`.

Part of #994.
oliverklee added a commit that referenced this issue Feb 25, 2025
@oliverklee
[TASK] Move getAllDeclarationBlocks up to CSSBlockList
013250c 
Move this method up without any changes to the method or its
callers.

We'll clean this up in later changes.

Part of #994.
@oliverklee oliverklee mentioned this issue Feb 25, 2025
Merged
oliverklee added a commit that referenced this issue Feb 25, 2025
@oliverklee
[TASK] Move up getAllDeclarationBlocks to CSSBlockList
3f7a428 
Move this method up without any changes to the method or its
callers.

We'll clean this up in later changes.

Part of #994.
oliverklee added a commit that referenced this issue Feb 26, 2025
@oliverklee
[TASK] Move up getAllDeclarationBlocks to CSSBlockList
ae0d883 
Move this method up without any changes to the method or its
callers.

We'll clean this up in later changes.

Part of #994.
oliverklee added a commit that referenced this issue Feb 26, 2025
@oliverklee
[CLEANUP] Refactor ::getAllDeclarationBlocks()
1529fd3 
Part of #994.
oliverklee added a commit that referenced this issue Feb 26, 2025
@oliverklee
[TASK] Move up getAllDeclarationBlocks to CSSBlockList
2255abc 
Move this method up without any changes to the method or its
callers.

We'll clean this up in later changes.

Part of #994.
JakeQZ pushed a commit that referenced this issue Feb 26, 2025
@oliverklee
[TASK] Move up getAllDeclarationBlocks to CSSBlockList (#996)
a1f9f58 
Move this method up without any changes to the method or its
callers.

We'll clean this up in later changes.

Part of #994.
oliverklee added a commit that referenced this issue Feb 26, 2025
@oliverklee
[CLEANUP] Refactor ::getAllDeclarationBlocks()
5709945 
Part of #994.
oliverklee added a commit that referenced this issue Feb 27, 2025
@oliverklee
[CLEANUP] Refactor ::getAllDeclarationBlocks()
83ec454 
Part of #994.
JakeQZ pushed a commit that referenced this issue Feb 27, 2025
@oliverklee
[CLEANUP] Refactor ::getAllDeclarationBlocks() (#990)
e1f8bd3 
Part of #994.
oliverklee added a commit that referenced this issue Mar 1, 2025
@oliverklee
[TASK] Move up getAllRuleSets to CSSBlockList
f72bc69 
Move this method up without any changes to the method or its callers.

We'll clean this up in later changes.

Part of #994.
@oliverklee oliverklee mentioned this issue Mar 1, 2025
Merged
JakeQZ pushed a commit that referenced this issue Mar 2, 2025
@oliverklee
[TASK] Move up getAllRuleSets to CSSBlockList (#1047)
f912e71 
Move this method up without any changes to the method or its callers.

We'll clean this up in later changes.

Part of #994.
oliverklee added a commit that referenced this issue Mar 2, 2025
@oliverklee
[CLEANUP] Refactor CSSBlockList::getAllRuleSets()
82667e2 
Also remove the now-unused method `allRuleSets()`.

Part of #994.
@oliverklee oliverklee mentioned this issue Mar 2, 2025
Merged
JakeQZ pushed a commit that referenced this issue Mar 2, 2025
@oliverklee
[CLEANUP] Refactor CSSBlockList::getAllRuleSets() (#1050)
54ca442 
Also remove the now-unused method `allRuleSets()`.

Part of #994.
JakeQZ added a commit that referenced this issue Apr 9, 2025
@JakeQZ
[CLEANUP] Remove CSSBlockList::allDeclarationBlocks()
7dcc6d2 
Change the one remaining usage instance to use `getAllDeclarationBlocks()`,
which was refactored in #990.

Part of #994.
@JakeQZ JakeQZ mentioned this issue Apr 9, 2025
Merged
oliverklee pushed a commit that referenced this issue Apr 9, 2025
@JakeQZ
[CLEANUP] Remove CSSBlockList::allDeclarationBlocks() (#1239)
0c94d6e 
Change the one remaining usage instance to use `getAllDeclarationBlocks()`,
which was refactored in #990.

Part of #994.
JakeQZ added a commit that referenced this issue Apr 10, 2025
@JakeQZ
[TASK] Move getAllValues() to CSSBlockList
47515c8 
Also add unit tests for this method.

Part of #994.  Relates to #1230.
@JakeQZ JakeQZ mentioned this issue Apr 10, 2025
Merged
JakeQZ added a commit that referenced this issue Apr 10, 2025
@JakeQZ
[TASK] Move getAllValues() to CSSBlockList
e3a9af9 
Also add unit tests for this method.

Part of #994.  Relates to #1230.
JakeQZ added a commit that referenced this issue Apr 10, 2025
@JakeQZ
[TASK] Move getAllValues() to CSSBlockList
2a59cc7 
Also add unit tests for this method.

Part of #994.  Relates to #1230.
oliverklee pushed a commit that referenced this issue Apr 10, 2025
@JakeQZ
[TASK] Move getAllValues() to CSSBlockList (#1240)
d9137fd 
Also add unit tests for this method.

Part of #994.  Relates to #1230.
JakeQZ added a commit that referenced this issue Apr 11, 2025
@JakeQZ
[TASK] Deconflate getAllValues() parameters
2477c9c 
The `$element` parameter was overloaded with a dual purpose.

A second separate parameter has been added for rule filtering,
which is not actually mutually exclusive with CSS subtree selection.

Since `getAllValues()` is part of the public API,
the method now supports being called with the old or new signatures,
with the old signature being deprecated.

Once the deprecation has been included in the 8.x release branch,
the messiness of supporting the previous API can be removed.

Part of #994.  Also relates to #1230.
@JakeQZ JakeQZ mentioned this issue Apr 11, 2025
Merged
oliverklee pushed a commit that referenced this issue Apr 11, 2025
@JakeQZ
[TASK] Deconflate getAllValues() parameters (#1241)
0119435 
The `$element` parameter was overloaded with a dual purpose.

A second separate parameter has been added for rule filtering,
which is not actually mutually exclusive with CSS subtree selection.

Since `getAllValues()` is part of the public API,
the method now supports being called with the old or new signatures,
with the old signature being deprecated.

Once the deprecation has been included in the 8.x release branch,
the messiness of supporting the previous API can be removed.

Part of #994.  Also relates to #1230.
JakeQZ added a commit that referenced this issue Apr 12, 2025
@JakeQZ
[TASK] Remove original getAllValues() API
3f7f281 
The method still exists with the same (slightly improved) functionality,
but the optional arguments have been refactored, and may require changes.

Part of #994.  Closes #1230.
@JakeQZ JakeQZ mentioned this issue Apr 12, 2025
Merged
oliverklee pushed a commit that referenced this issue Apr 12, 2025
@JakeQZ
[TASK] Remove original getAllValues() API (#1243)
4429cc2 
The method still exists with the same (slightly improved) functionality,
but the optional arguments have been refactored, and may require changes.

Part of #994.  Closes #1230.
JakeQZ added a commit that referenced this issue Apr 13, 2025
@JakeQZ
[TASK] Refactor getAllValues()
4bcd15c 
Move functionality from `allValues()` directly into to `getAllValues()`,
so as to avoid passing array by reference, removing `allValues()`.
Avoid making the recursive call for nested items that are not `CSSElement`s.

Part of #994.
@JakeQZ JakeQZ mentioned this issue Apr 13, 2025
Merged
JakeQZ added a commit that referenced this issue Apr 13, 2025
@JakeQZ
[TASK] Refactor getAllValues()
70b9ce7 
Move functionality from `allValues()` directly into to `getAllValues()`,
so as to avoid passing array by reference, removing `allValues()`.
Avoid making the recursive call for nested items that are not `CSSElement`s.

Part of #994.  Closes #1230.
JakeQZ added a commit that referenced this issue Apr 13, 2025
@JakeQZ
[TASK] Refactor getAllValues()
c02b02b 
Move functionality from `allValues()` directly into to `getAllValues()`,
so as to avoid passing array by reference, removing `allValues()`.
Avoid making the recursive call for nested items that are not `CSSElement`s.

Part of #994.
Closes #1230.
oliverklee pushed a commit that referenced this issue Apr 13, 2025
@JakeQZ
[TASK] Refactor getAllValues() (#1244)
03ed64a 
Move functionality from `allValues()` directly into to `getAllValues()`,
so as to avoid passing array by reference, removing `allValues()`.
Avoid making the recursive call for nested items that are not `CSSElement`s.

Part of #994.
Closes #1230.
JakeQZ added a commit that referenced this issue Apr 13, 2025
@JakeQZ
[CLEANUP] allSelectors() -> getAllSelectors()
ac5aec3 
The renamed (internal) method now returns the result,
instead of having a reference parameter for it.

Closes #994.
@JakeQZ JakeQZ mentioned this issue Apr 13, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oliverklee oliverklee

Labels
None yet
Projects
None yet
Milestone
8.9.0: Deprecations and bug fixes
Development

Successfully merging a pull request may close this issue.

[CLEANUP] allSelectors() -> getAllSelectors() MyIntervals/PHP-CSS-Parser
1 participant
@oliverklee