Skip to content

[DomCrawler] add a normalizeWhitespace argument to text() method #32440

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

Merged
merged 1 commit into from
Sep 27, 2019

Conversation

Simperfit
Copy link
Contributor

@Simperfit Simperfit commented Jul 8, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #21302, #23626
License MIT
Doc PR todo

I've taken #24412, rebased with 4.4, fixed the comments in the PR.

cc @xabbuh @stof @nicolas-grekas

@Simperfit
Copy link
Contributor Author

Simperfit commented Jul 8, 2019

The contribution header is the same as the other PR, should it be updated too ?

Edit: updated

@Simperfit Simperfit changed the title Issue 21302 rebased updated [DomCrawler] add a textContent() method, normalize whitespaces Jul 8, 2019
@Simperfit Simperfit force-pushed the issue-21302-rebased-updated branch 2 times, most recently from f0531dd to 88e79ff Compare July 9, 2019 07:31
@Simperfit Simperfit force-pushed the issue-21302-rebased-updated branch 2 times, most recently from c33f8ca to 9526c94 Compare July 9, 2019 07:57
@nicolas-grekas nicolas-grekas added this to the next milestone Jul 10, 2019
@Simperfit Simperfit force-pushed the issue-21302-rebased-updated branch from 9526c94 to ccce091 Compare July 11, 2019 04:04
@Simperfit Simperfit force-pushed the issue-21302-rebased-updated branch 2 times, most recently from 37fc6ad to 0b9ee0c Compare July 31, 2019 17:21
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.

please add a few test cases

@Simperfit Simperfit changed the title [DomCrawler] add a textContent() method, normalize whitespaces [DomCrawler] add a normalizeWhitespace argument tu text() method Aug 7, 2019
@Simperfit Simperfit force-pushed the issue-21302-rebased-updated branch from 0b9ee0c to 0a1a253 Compare August 7, 2019 05:51
@Simperfit Simperfit force-pushed the issue-21302-rebased-updated branch from 0a1a253 to f4d63fc Compare August 7, 2019 05:52
@Simperfit Simperfit changed the title [DomCrawler] add a normalizeWhitespace argument tu text() method [DomCrawler] add a normalizeWhitespace argument to text() method Aug 7, 2019
@nicolas-grekas
Copy link
Member

Please update the test case to ensure double spaces+line breaks in the middle of the text are normalized too.

@fabpot
Copy link
Member

fabpot commented Sep 23, 2019

@Simperfit Are you able to finish this PR?

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.

(I updated the tests)

@nicolas-grekas
Copy link
Member

Thank you @Simperfit.

@nicolas-grekas nicolas-grekas force-pushed the issue-21302-rebased-updated branch from f4d63fc to 1746718 Compare September 27, 2019 17:07
nicolas-grekas added a commit that referenced this pull request Sep 27, 2019
…t() method (Simperfit)

This PR was merged into the 4.4 branch.

Discussion
----------

[DomCrawler] add a normalizeWhitespace argument to text() method

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #21302, #23626  <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | todo <!-- required for new features -->

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

I've taken #24412, rebased with 4.4, fixed the comments in the PR.

cc @xabbuh @stof @nicolas-grekas

Commits
-------

1746718 [DomCrawler] add a value() method, normalize whitespaces
@nicolas-grekas nicolas-grekas merged commit 1746718 into symfony:4.4 Sep 27, 2019
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
@Simperfit Simperfit deleted the issue-21302-rebased-updated branch October 28, 2019 09:16
This was referenced Nov 12, 2019
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.

7 participants