Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add info about image into rss item. #155

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Add info about image into rss item. #155

merged 5 commits into from
Jun 13, 2024

Conversation

logart
Copy link
Contributor

@logart logart commented Jun 12, 2024

Hi, @w3stling

I really appreciate your library since it saved me a lot of time and I really like the extension points with addItemExtension API.
However, for my project I needed to parse media:thumbnail attribute of the rss feed. Of course I could parse it into author or comments field as a hack like this .addItemExtension("media:thumbnail", "url", Item::setAuthor), but I would rather contribute to the library to add additional image field on the rss Item class.

If you don't mind, I've added the default behaviour to abstract parser so it will work out of the box. But if this is inapropriate, I can happily use .addItemExtension("media:thumbnail", "url", Item::setImage) in my code.

Let me know if you could accept my PR as is, or should I remove default behaviour from abstact parser and only add an image field on the item class.

I wish you have a best day,
Artem

Modify abstract reader to parse image info by default.
@w3stling
Copy link
Owner

Thanks @logart for the PR.
I prefer not to add fields to the Item class that is not part of the RSS specification.

The easiest solution is as you suggested to map the media:thumbnail url attribute to author field. An alternative solution is to create your own RssReader class by extending AbstractRssReader class and override the createItem(DateTimeParser dateTimeParser) method. In that way you can add as many custom field to your implementation of the Item class. See ItunesRssReader class for an example.

I have started to implement a Media RSS module similar to the iTunes module that supports all tags and attributes of the Media RSS specification but I never finished that work.

@logart
Copy link
Contributor Author

logart commented Jun 13, 2024

Thanks, @w3stling This sounds like a great plan.

Do you think it's ok to add this custom reader to your repo like the iTunes one? I will gladly move implementation here to not pollute the Item class.

It's totally ok for me to maintain my own fork, but if you think this is a good solution I would rather merge it to your repository to not create a lot of copies of the library.

Let me know what do you think.

Best regards,
Artem

artem.loginov added 2 commits June 13, 2024 13:27
Add parser for media rss.
Add media rss extension on item.
Add support for parsing fields of media thumbnail.
@logart
Copy link
Contributor Author

logart commented Jun 13, 2024

I've updated the MR with media:thumbnail parsing support from media rss. Unfortunately I am unable to complete the whole spec. Hopefully when someone else would need it he can finish the job.
Thanks for your help. I really like your library.

@w3stling w3stling merged commit 459290e into w3stling:master Jun 13, 2024
3 of 4 checks passed
@w3stling
Copy link
Owner

Thanks @logart. Merged

@w3stling w3stling added the enhancement New feature or request label Jun 13, 2024
@logart
Copy link
Contributor Author

logart commented Jun 14, 2024

Thank you @w3stling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants