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

Update ASCII.php for PHP 8.4 compatibility #105

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Conversation

gilbertoalbino
Copy link
Contributor

Fixes #

php 8.4 : Implicitly marking parameter $replace_single_chars_only as nullable is deprecated, the explicit nullable type must be used instead

Proposed Changes

Change NULL for FALSE and update the PHPDocs.

@crynobone crynobone mentioned this pull request Nov 19, 2024
@voku voku merged commit 74c3ce8 into voku:master Nov 21, 2024
6 of 16 checks passed
@voku
Copy link
Owner

voku commented Nov 21, 2024

Thx for the fix. 👍

@TimWolla
Copy link

FWIW: This is a breaking change to the API. Explicitly passing null to the $replace_single_chars_only parameter will now result in a TypeError.

@phildawson-gravity
Copy link

I would agree with @TimWolla that changing to false is not the correct solution as it'll break those passing null.

It just needed to be marked as nullable with a question mark

?bool $replace_single_chars_only = null

@gilbertoalbino
Copy link
Contributor Author

Hi @phildawson-gravity. This package is supposed to support PHP 7.0+.
Nullable types were added in PHP 7.1.

@phildawson-gravity
Copy link

phildawson-gravity commented Nov 22, 2024

@gilbertoalbino I hadn't appreciated that, I understand keeping support for the legacy 7.4, but 7.0 came out almost nine years ago and ended support five years ago.

Anyone passing null will just need to adapt to false to deal with it.

@gilbertoalbino
Copy link
Contributor Author

@phildawson-gravity that anyone you mention does happen to just be Laravel until now, but they have done that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants