Remove default value for parameter followed by required parameter

[Stadly]

Parameter with default value followed by required parameter is deprecated in PHP 8. Just removing the default values should not lead to a change in functionality. Ref

[Stadly]

I just realized there is another pull request for this. I think removing the default values is a better fix than adding one for the last parameter, as the key drop_links is assumed to exist in $options, and therefore an empty array is not a good default value for that parameter.

Anyhow, one of these fixes should be merged as soon as possible to get rid of the deprecation notice in PHP 8.

[AdeAttwood]

It is worth pointing out there is no visibility on the method and by default this is public. This PR is a braking change and would need to be bumped to v2.0.0.

Having said that it is not in the docs and an internal function. Either way it makes no difference to me, I am with @Stadly and the priority should be getting PHP8 support. We are using Yii2, and they promote warnings to exceptions which makes this is a bigger issue.

[Stadly]

I don't think this is a breaking change. Calling iterateOverNode($node) would, assuming that execution is not stopped due to missing arguments, result in the following values:

Variable	v1.1.0	This PR
$prevName	null	null
$in_pre	false	null
$is_office_document	false	null
$options	null	null

$in_pre is used on lines 234 and 352. Both places just checks the truthness of the variable, so false and null will give the same result.

$is_office_document is used on line 305. Only the truthness of the variable is checked, so false and null will give the same result.

Therefor iterateOverNode will work the same with this PR, regardless of the number of arguments provided.

To my understanding, removing the defaults should not introduce any issues for classes extending the class.

Hence there should be no BC break. Or am I missing something?

[AdeAttwood]

@Stadly you are correct it was me that was missing something. With the last parameter not having a default value you need to pass in all the parameters, all existing implementations will need to be passing all parameters in.

I am with you and calling iterateOverNode($node) does not make sense. I will close my PR if favour of this one. My PR will need refactoring to move the defaultOptions into the iterateOverNode method and I don't fancy doing that.

[Lozik]

@soundasleep I would like to express my support for this PR and, as this would fix PHP8 support, it should be merged ASAP, if you please.

[obstschale]

Any Update on this PR?

[morungos]

I could use this PR if there's a chance, this is the one thing stopping me from switching to php8

[dees040]

I made a temporary fork of this PR so that I can continue running this package on my PHP 8 project. Might be useful for some of you too.

composer remove soundasleep/html2text
composer require dees040/html2text