Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DomCrawler] Added argument $default to method Crawler::attr() #51368

Merged
merged 1 commit into from Aug 16, 2023

Conversation

Rastishka
Copy link
Contributor

Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets no
License MIT
Doc PR symfony/symfony-docs#...

Added argument $default to method ->attr() like ->text() and ->html() have.

@nicolas-grekas
Copy link
Member

Do we really need this? My feeling is that we now have the ?? operator and it's nicer to use.

@antonkomarev
Copy link

antonkomarev commented Aug 12, 2023

I'd prefer to use native ??. It feels more obvious about the result.

@Rastishka
Copy link
Contributor Author

Do we really need this? My feeling is that we now have the ?? operator and it's nicer to use.

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:

try {
    $value = $dom->filter('a.someclass')->attr('href');
} catch (\InvalidArgumentException $e) {
    $value = '';
}

can be simplifies to
$value = $dom->filter('a.someclass')->attr('href', '');

Copy link
Member

@nicolas-grekas nicolas-grekas left a 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.

src/Symfony/Component/DomCrawler/Crawler.php Outdated Show resolved Hide resolved
@nicolas-grekas
Copy link
Member

Please also update the UPGRADE-6.4.md file, and please rebase to get rid of merge commits (PRs shouldn't contain any).

@nicolas-grekas nicolas-grekas changed the title Added argument $default to method ->attr() like ->text() and ->html() have. Added argument $default to method Crawler::attr() Aug 16, 2023
@carsonbot carsonbot changed the title Added argument $default to method Crawler::attr() [DomCrawler] Added argument $default to method Crawler::attr() Aug 16, 2023
@nicolas-grekas
Copy link
Member

Thank you @Rastishka.

@nicolas-grekas nicolas-grekas merged commit c2ac73c into symfony:6.4 Aug 16, 2023
7 of 9 checks passed
This was referenced Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants