feat: Add media content parsing for images#158
feat: Add media content parsing for images#158DoomHammer wants to merge 1 commit intocustom-components:masterfrom
Conversation
DoomHammer
force-pushed
the
add-media-content-parsing
branch
from
aee4864 to
5eab987
Compare
ogajduse
left a comment
ogajduse
left a comment
There was a problem hiding this comment.
Thank you for your patch. I left one blocking comment. I'd also ask you to cover this behavior with one or more tests.
| if images: | ||
| # pick the first image found | ||
| return images[0]["url"] | ||
| elif "summary" in feed_entry: |
There was a problem hiding this comment.
If there is no media_content, we'd still like this logical branch to be executed. Or if media_content exists but contains no images, the summary fallback will be skipped.
| elif "summary" in feed_entry: | |
| if "summary" in feed_entry: |
There was a problem hiding this comment.
Pull request overview
Adds support for parsing images from media_content entries in RSS/Atom items, improving compatibility with feeds like Mastodon that publish images via media extensions.
Changes:
- Extend
_process_imageto look forimage/*items infeed_entry["media_content"]. - Return the first matching
media_contentimage URL when present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| images = [ | ||
| enc for enc in feed_entry["media_content"] if enc['type'].startswith("image/") | ||
| ] |
There was a problem hiding this comment.
Filtering media_content items with enc['type'].startswith("image/") can raise KeyError (missing type) or AttributeError (type is None). Using a safe accessor (e.g., enc.get("type", "")) and matching the enclosures style (enc.type) would make this more robust for varying feedparser outputs.
There was a problem hiding this comment.
I consider this a minor comment.
| if feed_entry.get("media_content"): | ||
| images = [ | ||
| enc for enc in feed_entry["media_content"] if enc['type'].startswith("image/") | ||
| ] | ||
| if images: | ||
| # pick the first image found | ||
| return images[0]["url"] |
There was a problem hiding this comment.
This change introduces a new image source (media_content) but the test suite doesn't include any fixture/feed data that exercises media_content image parsing. Adding a representative feed sample (e.g., Mastodon) and asserting the selected image URL would prevent regressions and validate the new behavior.
| enc for enc in feed_entry["media_content"] if enc['type'].startswith("image/") | ||
| ] | ||
| if images: | ||
| # pick the first image found | ||
| return images[0]["url"] | ||
| elif "summary" in feed_entry: |
There was a problem hiding this comment.
The summary fallback is skipped when media_content exists but contains no image items because this uses elif "summary" in feed_entry after the media_content block. This can cause DEFAULT_THUMBNAIL to be returned even when an <img> exists in the summary. Consider making the summary check an independent if (or otherwise falling through) so it runs when media_content has no usable images.
| enc for enc in feed_entry["media_content"] if enc['type'].startswith("image/") | |
| ] | |
| if images: | |
| # pick the first image found | |
| return images[0]["url"] | |
| elif "summary" in feed_entry: | |
| enc for enc in feed_entry["media_content"] if enc["type"].startswith("image/") | |
| ] | |
| if images: | |
| # pick the first image found | |
| return images[0]["url"] | |
| if "summary" in feed_entry: |
Add media_content parsing for images. Useful for Mastodon feeds.