Skip to content

Conversation

@tacman
Copy link

@tacman tacman commented Aug 21, 2025

allow php-css-parser 9

all php-css-parser 9
@bsweeney
Copy link
Member

bsweeney commented Aug 21, 2025

Unit tests are happy. Have you had a chance to review changes between 8 and 9? I haven't yet done so to see if there are any breaking changes that need to be addressed.

@tacman
Copy link
Author

tacman commented Aug 21, 2025

phpstan reports the same warnings with both 8 and 9. They should probably be fixed regardless. The signatures are the same, so I imagine they are compatible.

I'm hesitant to fix these, though, because you're still supporting PHP 7.1, and maintaining compatibility with that is awkward. If yo ever release a new version, I'd recommend rector and updating to only supported versions of php.

Very low priority, I just noticed the new version of that library.

phpstan analyze src
29/29 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


Line Svg/Style.php


89 Method Svg\Style::fromAttributes() should return Svg\Style but return statement is missing.
🪪 return.missing
91 Method Svg\Style::fromAttributes() should return Svg\Style but return statement is missing.
🪪 return.missing



Line Svg/Surface/CPdf.php


1109 Undefined variable: $object
🪪 variable.undefined
1111 Undefined variable: $object
🪪 variable.undefined
3624 Call to vsprintf contains 3 placeholders, 4 values given.
🪪 argument.vsprintf
3663 Call to vsprintf contains 3 placeholders, 4 values given.
🪪 argument.vsprintf



Line Svg/Surface/SurfaceCpdf.php


220 Call to an undefined method Svg\Surface\SurfaceCpdf::_convert_gif_bmp_to_png().
🪪 method.notFound


@JakeQZ
Copy link

JakeQZ commented Nov 6, 2025

Unit tests are happy. Have you had a chance to review changes between 8 and 9? I haven't yet done so to see if there are any breaking changes that need to be addressed.

The only potentially breaking change is MyIntervals/PHP-CSS-Parser#1194. It is unlikely to be breaking, but since it changed the class hierarchy, we considered it worthy of a new major release, just in case.

You can allow v8 or v9 in composer.json. I hope you can resolve this soon - we have had a PR attempting to add PHP 8.5 support to the 8.x branch which is no longer supported. The 9.x version is compatible with PHP from 7.2, which should be sufficient.

@bsweeney
Copy link
Member

bsweeney commented Nov 6, 2025

Thank you for the information. I do plan to start reviewing changes for the next release of SvgLib i the near future.

@tacman
Copy link
Author

tacman commented Nov 6, 2025

next release of SvgLib i

What about dropping all unsupported/EOL versions of the dependencies?

@bsweeney
Copy link
Member

bsweeney commented Nov 6, 2025

I'm fine to do that, though preferably separately as a major release. I know it's not strictly necessary since dependency management will take care of it, but I prefer to do it that way to maintain a clear cut off point.

@tacman
Copy link
Author

tacman commented Nov 6, 2025

Makes sense. PHP 7.1 reached EOL in 2019. PHP 8.5 will be released in a few weeks, seems like a nice time for a new release.

Copy link

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

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

Looks good to me.

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.

Update to sabberworm/php-css-parser v9

4 participants