Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DomCrawler] Added argument$default to methodCrawler::attr()#51368
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedAug 12, 2023
Do we really need this? My feeling is that we now have the |
antonkomarev commentedAug 12, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'd prefer to use native |
Rastishka commentedAug 12, 2023
Yes we need, because setting default value also prevents throwing exception when node does not exist (methods text() and html() act similar). So this code: can be simplifies to |
nicolas-grekas left a comment
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.
Thanks for the explanation. Works for me.
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedAug 14, 2023
Please also update the UPGRADE-6.4.md file, and please rebase to get rid of merge commits (PRs shouldn't contain any). |
$default to method->attr() like->text() and->html() have.$default to methodCrawler::attr()$default to methodCrawler::attr()$default to methodCrawler::attr()nicolas-grekas commentedAug 16, 2023
Thank you@Rastishka. |
Added argument
$defaultto method->attr()like->text()and->html()have.