Conversation
|
Btw. I think there is a lot of dead code? The Url class is never used, nor the classmethods |
| has_width = bool(width) | ||
| height = options.get("height", None) | ||
| has_height = bool(height) |
There was a problem hiding this comment.
Hey @Speedy1991, would it be better to use has_width = width is not None instead of has_width = bool(width)? It might handle cases like 0 more clearly.
And will be not necessary to include or 0
|
I'll check this PR again in 1-2 weeks after my vacation. I'm still not sure if there is more refactor needed, depending the dead code |
|
This PR is stale because it has been open 45 days with no activity. Remove the stale label or add a comment, or this PR will be closed in 10 days. You can always re-open if you feel this is something we should still keep working on. Tag @heynemann for more information. |
There was a problem hiding this comment.
Pull request overview
Fixes libthumbor’s URL generation so explicitly passing width=0 and/or height=0 preserves the 0x0 dimensions segment (preventing Thumbor filters from being ignored), and updates repo ignores for IDE metadata.
Changes:
- Treat
width=0/height=0as explicitly provided dimensions (emit0x0) instead of dropping the dimension segment. - Update
Url.generate_options()defaults toNoneand adjust dimension appending logic accordingly. - Add tests for
width=0/height=0URL composition and ignore.ideain.gitignore.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
libthumbor/url.py |
Updates dimension handling to preserve 0x0 when explicitly requested; adjusts Url.generate_options() defaults and dimension emission. |
tests/test_url_composer.py |
Adds regression tests for width=0 / height=0 URL generation. |
.gitignore |
Ignores .idea directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| def test_url_height_0_url_with_0(): | ||
| url = url_for(height=0, width=0, image_url=IMAGE_URL) | ||
| expect(url).to_equal("84996242f65a4d864aceb125e1c4c5ba") |
| if horizontal_flip: | ||
| width = f"-{width}" | ||
| if vertical_flip: | ||
| height = f"-{height}" | ||
|
|
||
| if width or height: | ||
| url.append(f"{width}x{height}") | ||
| if width is not None or height is not None: | ||
| url.append(f"{ width or 0}x{height or 0}") |
This will fix #60
I also added the .idea to gitignore