Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
[DomCrawler] Add more details about some methods#21034
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should probably also make it clear which methods are meant to be called on a single-node Crawler because they ignore other nodes than the first one (any method where the implementation rely on using$this->getNode(0) basically).
We have several places guaranteed to be dealing with single-node crawlers in the API:
- the callback of
each - the callback of
reduce - the return value of
eq(and its shortcut) for a valid position (for an invalid position, it will be an empty crawler for which those methods relying on the first node will throw an exception)
testing/dom_crawler.rst Outdated
| ``first()`` | ||
| Firstnode. | ||
| Selects the firstnode (equivalent to ``eq(0)``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
UsingSelects in all those descriptions is confusing to me. We have different kinds of methods in this lists:
- methods creating a Crawler containing only a subset of the nodes of the current Crawler instance (
eqand all its shorcuts, but alsoreducewhich actually filters the current instance andslice) - methods returning a new Crawler with a different selection (whichalso includes the
filterandfilterXpathmethods, despite their name, as they are doing a mix of matching the existing nodes or finding their descendants)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You are right, but this doc is intesting/dom_crawler (we also havecomponents/dom_crawler) so this is probably an internal detail not relevant in this page. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Well, if you improve the documentation of available methods to make it easier to know what they do (without going to another page), it is relevant to make it clear what they do IMO. Otherwise, you could remove the list and make them go tocomponents/dom_crawler to get the proper explanation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I've (slightly) updated all the descriptions to be more precise. Thanks!
Uh oh!
There was an error while loading.Please reload this page.
fa468e4 intosymfony:6.4Uh oh!
There was an error while loading.Please reload this page.
After merging#21031 I thought that we could improve some explanations of some methods with more details and references.