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.

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

[DomCrawler] Add argument $normalizeWhitespace to Crawler::innerText() and make it return the first non-empty text #48940

Merged
merged 1 commit into from Jan 11, 2023
Merged

[DomCrawler] Add argument $normalizeWhitespace to Crawler::innerText() and make it return the first non-empty text #48940

merged 1 commit into from Jan 11, 2023

Conversation

otsch
Copy link
Contributor

@otsch otsch commented Jan 10, 2023

This is a new PR instead of #48684 with target branch 6.3 as requested.

Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #48682
License MIT

@carsonbot carsonbot added this to the 6.3 milestone Jan 10, 2023
@OskarStark OskarStark changed the title Bugfix/issue 48682 [DomCrawler] Fix getting text after a child node with Crawler::innerText() Jan 10, 2023
@OskarStark OskarStark mentioned this pull request Jan 10, 2023
Closed
@OskarStark OskarStark requested review from nicolas-grekas and fabpot and removed request for nicolas-grekas January 10, 2023 13:21
@fabpot fabpot changed the title [DomCrawler] Fix getting text after a child node with Crawler::innerText() [DomCrawler] Allow to trim text in Crawler::innerText() Jan 10, 2023
fabpot
fabpot requested changes Jan 10, 2023
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to follow the instructions I've given in the other PR to avoid having a BC break.

@carsonbot carsonbot changed the title [DomCrawler] Allow to trim text in Crawler::innerText() Allow to trim text in Crawler::innerText() Jan 10, 2023
@otsch
Copy link
Contributor Author

otsch commented Jan 10, 2023

You need to follow the instructions I've given in the other PR to avoid having a BC break.

@fabpot Ah OK, you mean like this? 88a2575

@otsch otsch force-pushed the bugfix/issue-48682 branch 2 times, most recently from ad1ee7d to 9c08134 Compare January 10, 2023 14:02
@otsch
Copy link
Contributor Author

otsch commented Jan 10, 2023

@fabpot sorry I just ammended the commit: 9c08134 to have it exactly as you explained. But the fabbot.io action doesn't like the @param phpdoc: https://fabbot.io/report/symfony/symfony/48940/9c08134769a558074bcb3c5ac20eda57ffd663a4
I also tried adding the text parameter not implemented yet like it's also in other places with commented method arguments, but it still complains 🤔

@otsch otsch requested review from fabpot and removed request for nicolas-grekas January 10, 2023 14:05
@carsonbot carsonbot changed the title Allow to trim text in Crawler::innerText() [DomCrawler] Allow to trim text in Crawler::innerText() Jan 10, 2023
@nicolas-grekas nicolas-grekas changed the title [DomCrawler] Allow to trim text in Crawler::innerText() [DomCrawler] Add argument $normalizeWhitespace to Crawler::innerText() and make it return the first non-empty text Jan 11, 2023
@otsch @nicolas-grekas
[DomCrawler] Add argument $normalizeWhitespace to `Crawler::innerTe…
bb0c214 
…xt()` and make it return the first non-empty text
@fabpot
Copy link
Member

fabpot commented Jan 11, 2023

Thank you @otsch.

@fabpot fabpot mentioned this pull request Jan 11, 2023
Closed
@fabpot fabpot merged commit 991c2ba into symfony:6.3 Jan 11, 2023
@otsch otsch deleted the bugfix/issue-48682 branch January 11, 2023 19:06
otsch added a commit to otsch/symfony-docs that referenced this pull request Jan 12, 2023
@otsch
Adapt docs for Crawler::innerText()
e5047ee 
According to the changes from
symfony/symfony#48940
@otsch otsch mentioned this pull request Jan 12, 2023
Merged
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull request Jan 13, 2023
@javiereguiluz
minor symfony#17729 [DomCrawler] Adapt docs for `Crawler::innerText()…
982c8d5 
…` (otsch)

This PR was merged into the 6.3 branch.

Discussion
----------

[DomCrawler] Adapt docs for `Crawler::innerText()`

According to the changes from symfony/symfony#48940
And as requested in symfony#17722

Commits
-------

e5047ee Adapt docs for Crawler::innerText()
@fabpot fabpot mentioned this pull request May 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bilge Bilge Bilge left review comments

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Bug DomCrawler Status: Reviewed
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.

DomCrawler component Crawler::innerText() doesn't get text after a child node
5 participants
@otsch @fabpot @nicolas-grekas @Bilge @carsonbot