python-needle / needle

Automated tests for your CSS.
https://needle.readthedocs.io/
Other
590 stars 50 forks source link

General external comparison tool support #5

Closed acdha closed 10 years ago

acdha commented 11 years ago

We should probably have a simple configurable way to use external comparison tools like the ImageMagick/GraphicsMagick convert utilities. The current support for perceptualdiff could be replaced with this, which would also provide a way to customize arguments.

jphalip commented 10 years ago

I've posted a suggested fix for this in #19. Any thoughts?

acdha commented 10 years ago

At first glance that looks nice, particularly using a class name so we could start pushing more arcane things out into external plugins. I was thinking about adding one using the Image/GraphicsMagick compare utility & wondering what the threshold should be for splitting diff.py into a directory.

jphalip commented 10 years ago

What arcane things are you suggesting to push out?

Also, are you suggesting to split diff.py into a directory like https://github.com/mariocesar/sorl-thumbnail/tree/master/sorl/thumbnail/engines ?

acdha commented 10 years ago

“arcane things” really just meant that if you want to experiment with something new, you can do so in a separate project rather than having to run from a fork. Since e.g. ImageMagick's compare function accepts a ton of different options, there's a certain argument that it might best be developed externally.

What sorl-thumbnail is doing looks exactly like what I had in mind.

jphalip commented 10 years ago

Yes, I totally agree, using the class path allows anyone to plug whatever engine they want.

I'll rearrange diff.py into a directory to make things a bit more organized.

jphalip commented 10 years ago

I've updated the pull request with the new directory structure. What do you think?

jphalip commented 10 years ago

Quick note that unless there's any objection or further feedback I'll merge this PR tomorrow. I'll then try and add more documentation in preparation for a new release soon as well.

bfirsh commented 10 years ago

Looks really great :)

acdha commented 10 years ago

I overlooked your recent changes. Looks great to me - merge away!

jphalip commented 10 years ago

Great, thanks for the feedback!

jphalip commented 10 years ago

Fixed in b308ac07f27e588420a07dfa6028520d3b459d96