search

michelf / php-markdown

Parser for Markdown and Markdown Extra derived from the original Markdown.pl by John Gruber.
http://michelf.ca/projects/php-markdown/
Other
3.42k stars 530 forks source link

preg_split returning false will cause TypeError on invalid countable in PHP 8 #379

Closed sanmai closed 2 years ago

michelf commented 2 years ago

Thank you.

sanmai commented 2 years ago

@michelf Thanks for considering this PR! Can we have a new release ready with this and other changes?

I'll be happy to give it a go and report back with any new issues.

I could also give you a hand with anything else you need to get it moving.

michelf commented 2 years ago

I guess what remains would be to pick a version number, and write the change log in Readme.md.

I should be able to do that by tomorrow.

sanmai commented 2 years ago

I reviewed the changes since the last release, and it looks like the plan you had with 2.0 is a fair bet: there are properties that could no longer accept mixed types, which looks like a BC break to me.

michelf commented 2 years ago

Oh, didn't think about that actually. I was planning on deciding between 1.9.2 and 1.10.0 after looking at the changes... but changing the major version just for type annotations makes me rethink whether those type annotations are really worth the trouble. 🤔

sanmai commented 2 years ago

It works like this. If a caller has declare(strict_types = 1); where they use this library, if they assign an inappropriate value to a strictly typed property:

$obj = new \Michelf\MarkdownExtra();
$obj->no_markup = 1;

They will get a TypeError: Cannot assign int to property Michelf\Markdown::$no_markup of type bool so although it's a problem on the caller's side (they could remove strict_types declaration), they couldn't simply composer update and expect everything to work as before. Therefore, unfortunately this looks like a breaking change warranting a new major version.

sanmai commented 2 years ago

To be clear, I don't have anything against a new major version. It is something that has to be done, that's it.