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

Allow null for nl2br #3619

Merged
merged 1 commit into from Jan 3, 2022
Merged

Allow null for nl2br #3619

merged 1 commit into from Jan 3, 2022

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Jan 3, 2022

Same as #3617
Fixes Passing null to parameter #1 ($string) of type string is deprecated on PHP 8.1

@ruudk ruudk force-pushed the nl2br branch 2 times, most recently from 9eaccfa to 690602e Compare January 3, 2022 12:49
@ruudk
Copy link
Contributor Author

ruudk commented Jan 3, 2022

@fabpot ready for review. The failing test is not related.

tests/Fixtures/filters/nl2br.test Outdated Show resolved Hide resolved
tests/IntegrationTest.php Show resolved Hide resolved
@@ -231,7 +231,7 @@ public function getFilters()
new TwigFilter('lower', 'twig_lower_filter', ['needs_environment' => true]),
new TwigFilter('striptags', 'strip_tags'),
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose we have the same issue with this filter? And maybe all the other ones that call PHP's native function directly with no wrappers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the following:

  • sprintf
  • strip_tags

Should we also do something with abs? When null, pass 0?

@ruudk ruudk force-pushed the nl2br branch 2 times, most recently from 06527c8 to 0547ab1 Compare January 3, 2022 13:18
@ruudk
Copy link
Contributor Author

ruudk commented Jan 3, 2022

@fabpot Updated the PR, added missing test for striptags filter, and made sure that striptags and format filters now also accept null inputs.

src/Extension/CoreExtension.php Outdated Show resolved Hide resolved
src/Extension/CoreExtension.php Outdated Show resolved Hide resolved
@ruudk
Copy link
Contributor Author

ruudk commented Jan 3, 2022

Fixed

@fabpot
Copy link
Contributor

fabpot commented Jan 3, 2022

Thank you @ruudk.

@fabpot fabpot merged commit 9aa3845 into twigphp:2.x Jan 3, 2022
@ruudk ruudk deleted the nl2br branch January 3, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants