search

Intervention / image

PHP Image Processing
https://image.intervention.io
MIT License
13.92k stars 1.5k forks source link

Implemented basic ImageMagick trim modifier #1271

Closed seebeen closed 7 months ago

seebeen commented 9 months ago

Let's use this as a starting point. Looking forward to your comments @olivervogel

Will close #1239

olivervogel commented 9 months ago

Wow, that was fast. I will take a look later. 👀

seebeen commented 9 months ago

Works very well. I have only made a few minor comments about the PSR standard. Otherwise you can try to integrate the modifier with as good as it gets same functionality for the GD library, then I can merge it.

I avoid using PHPStan as the plague. Do you mind if I add-in PHPCS as a dev dependency with a PSR12 standard config? We can spend some time tweaking it afterward :)

olivervogel commented 9 months ago

I avoid using PHPStan as the plague. Do you mind if I add-in PHPCS as a dev dependency with a PSR12 standard config?

Do you want to add phpcs additionally or instead of phpstan?

seebeen commented 9 months ago

I avoid using PHPStan as the plague. Do you mind if I add-in PHPCS as a dev dependency with a PSR12 standard config?

Do you want to add phpcs additionally or instead of phpstan?

Add it as an additional dev dependency - wouldn't want to impose my workflow setup on you (or anyone else). People who use both - will get both. Others (like me) get to pick a camp 😊 Personally I use VSCode and Intelephpense - with custom phpcs rules.

olivervogel commented 9 months ago

Add it as an additional dev dependency

Ok sure. Create a new PR for this if you want.

seebeen commented 9 months ago

New PR created - #1275.

seebeen commented 9 months ago

Added the GD trim modifier. Converting this to proper PR, and awaiting your feedback 💪

olivervogel commented 8 months ago

I wanted to ask if you still plan to continue here? If you have any questions or uncertainties, please get in touch. If you're no longer interested, that's fine too, of course. I won't ask any more questions and will close the PR.

seebeen commented 8 months ago

Heya.

Yes, I do want to continue. I'll aim to finalize by monday.

joergmoenke commented 7 months ago

@olivervogel any chance to get this merged?

olivervogel commented 7 months ago

@olivervogel any chance to get this merged?

Yes, of course, unfortunately the current status does not yet meet my quality standards. The results with the modifer with the two drivers are too different and sometimes end up with errors. See my comment here: https://github.com/Intervention/image/pull/1271#discussion_r1465112599

olivervogel commented 7 months ago

I took the basis of @seebeen as a starting point and further developed the feature in this PR. It continues there and I am closing this one.