[DomCrawler] Add a way to filter direct children #28221
Conversation
0bc09b8 to
8c9e3c7
Compare
| * @throws \RuntimeException if the CssSelector Component is not available and $selector is provided | ||
| */ | ||
| public function children() | ||
| public function children($selector = null) |
There was a problem hiding this comment.
Actually, this is a BC break: child classes overriding this method would break with this change.
What we do in these situations is fetching the argument using \func_get_arg(), see e.g. the Finder::sortByName() method.
public function children(/* string $selector = null */) should be the new signature
| } | ||
|
|
||
| /** | ||
| * @param bool $html Whether HTML support should be enabled. Disable it for XML documents |
There was a problem hiding this comment.
this argument should be removed: read $this->isHtml instead.
| /** | ||
| * @param bool $html Whether HTML support should be enabled. Disable it for XML documents | ||
| * | ||
| 8000 * @return CssSelectorConverter A CssSelectorConverter instance |
There was a problem hiding this comment.
should be removed and replaced by a real return type on the method
| * | ||
| * @return CssSelectorConverter A CssSelectorConverter instance | ||
| * | ||
| * @throws \RuntimeException if the CssSelector Component is not available |
There was a problem hiding this comment.
uppercase "If" (same in other places)
|
Thank you very much for your feedbacks @nicolas-grekas :). |
| /** | ||
| * Returns the children nodes of the current selection. | ||
| * | ||
| * @param $selector string|null An optional CSS selector to filter children |
There was a problem hiding this comment.
swap needed: @param string|null $selector
| throw new \InvalidArgumentException('The current node list is empty.'); | ||
| } | ||
|
|
||
| if ($selector) { |
There was a problem hiding this comment.
if (null !== $selector) {?
| } | ||
|
|
||
| /** | ||
| * @throws \RuntimeException if the CssSelector Component is not available |
|
Thanks @nicolas-grekas :). Fixed. |
| $converter = $this->createCssSelectorConverter($this->isHtml); | ||
| $xpath = $converter->toXPath($selector, 'child::*/'); | ||
|
|
||
| return $this->filterXPath($xpath); |
There was a problem hiding this comment.
this could be made much more efficient by using child:: as prefix when converting CSS and then using filterRelativeXPath instead of going through relativize to modify the XPath again
|
@stof Thank you for your feedback. I fixed it. |
| * @throws \RuntimeException If the CssSelector Component is not available and $selector is provided | ||
| */ | ||
| public function children() | ||
| public function children(/* string $selector = null */) |
There was a problem hiding this comment.
@nicolas-grekas shouldn't we also detect cases where getclass($this) !== __CLASS__ (plus the special handling of common mock libraries) to trigger a deprecation warning if they don't have the argument in the child class ? Otherwise, we don't have a continous migration path.
There was a problem hiding this comment.
Looking at the source, we forgot such deprecations in many places.
Maybe merge as is (the continuous upgrade path would be just not adding this argument in v5)
and fix all the code base at once next?
6f303ec to
f634afd
Compare
|
Thank you @einenlum. |
…nlum) This PR was squashed before being merged into the 4.2-dev branch (closes #28221). Discussion ---------- [DomCrawler] Add a way to filter direct children | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28171 | License | MIT | Doc PR | - The Dom-Crawler component only has a `filter()` method (to filter the node and all its children) and a `children()` method to return direct children. **There is currently no way to easily filter (thanks to a selector) the direct children of a node, like jQuery allows so (with a selector passed to the `.children([selector])` method).** **This PR adds a way to optionally filter direct children thanks to a CSS selector**. Here is an example of the usage: ```php $html = <<<'HTML' <html> <body> <div id="foo"> <p class="lorem" id="p1"></p> <p class="lorem" id="p2"></p> <div id="nested"> <p class="lorem" id="p3"></p> </div> </div> </body> </html> HTML; $crawler = new Crawler($html); $foo = $crawler->filter('#foo'); $foo->children() // will select `#p1`, `#p2` and `#nested` $foo->children('p') // will select `#p1` and `p2` $foo->children('.lorem') // will select `#p1` and `p2` ``` This PR adds only an optional parameter and adds no BC break. Commits ------- f634afd [DomCrawler] Add a way to filter direct children
|
Thank you very much @nicolas-grekas! :) |
|
@einenlum thanks for this feature! We've created symfony/symfony-docs#10288 to not forget about documenting this new feature. It'd be great if you could provide the docs for it. If you need any help doing that, ask us in the Symfony Docs repository. Thanks! |
|
@javiereguiluz Thank you for your comment. I will create a PR on Symfony Docs in the following days, no problem! |
The Dom-Crawler component only has a
filter()method (to filter the node and all its children) and achildren()method to return direct children.There is currently no way to easily filter (thanks to a selector) the direct children of a node, like jQuery allows so (with a selector passed to the
.children([selector])method).This PR adds a way to optionally filter direct children thanks to a CSS selector. Here is an example of the usage:
This PR adds only an optional parameter and adds no BC break.