[WIP] Basic support for itunes rss tags #2674

anderbubble · 2017-02-19T09:58:43Z

I'm working on enhancing Nikola to support publishing a podcast RSS feed. Most of this work is going into a "postcast" plugin I'm working on, and will submit or publish eventually; but some of what I'm trying to do requires support in Nikola itself.

This PR aims to support iTunes-specific tags in RSS feeds.

Other related PRs I'm working on:

Provide copyright field in RSS feed #2676 (copyright tag in rss feeds)
[WIP] Optionally read rss guid from the post #2675 (explicit / static guids)

I'd also be interested in direction for where and how to publish a plugin, when that's ready.

todo

remove references to postcast

Kwpolska · 2017-02-19T10:09:48Z

nikola/utils.py

@@ -1272,13 +1286,66 @@ def publish_extensions(self, handler):
            })
            handler.endElement("atom:link")

+        if self.itunes_author:


You could duplicate less code, eg. with getattr or some iTunes data dictionary.

Kwpolska · 2017-02-19T10:11:34Z

Looks good (apart from a lot of duplicated code). Please update CHANGES.txt, AUTHORS.txt, and the manual.

Kwpolska · 2017-02-19T10:13:58Z

nikola/utils.py

+        self.itunes_categories = itunes_categories
+        self.itunes_explicit = itunes_explicit
+
+        return rss.RSS2.__init__(self, **kwargs)


Don’t use return here, instead call rss.RSS2.__init__ at the start of this function. (old-style class in Python 2, so super() does not work(

anderbubble · 2017-02-19T14:41:55Z

First, thank you for reviewing this pull request!

I returned __init__ from __init__ because the code for Item already did. Should I clean that up at the same time?

As for the duplicated code: can you provide a pointer to somewhere else in the codebase that does something similar better? (It'd just be nice to have an example of what idiom is considered most normal in this codebase.)

I'm also not happy with my handling of the Image url. Any suggestions for how to do it better?

Kwpolska · 2017-02-19T14:54:36Z

Yes, you can do that as well.
I don’t think we have anything like this. Here’s one example:

itunes = {'itunes:subtitle': 'foo'}
# in publish_extensions
for name in ['itunes:author', 'itunes:subtitle', 'itunes:summary', 'itunes:duration']:
    if name in itunes:
        handler.startElement(name, {})
        handler.characters(itunes[name])
        handler.endElement(name)
# special handling for image and explicit stays

You could ask url_replacer with url_type='absolute'.

felixfontein · 2017-02-19T16:44:17Z

nikola/nikola.py

@@ -1649,7 +1649,7 @@ def apply_shortcodes(self, data, filename=None, lang=None, with_dependencies=Fal

    def generic_rss_renderer(self, lang, title, link, description, timeline, output_path,
                             rss_teasers, rss_plain, feed_length=10, feed_url=None,
-                             enclosure=_enclosure, rss_links_append_query=None):
+                             enclosure=_enclosure, rss_links_append_query=None, itunes=None):


Souldn't the default value for itunes be False and not None? After all, it is only used like a boolean.

anderbubble · 2017-02-19T16:52:01Z

It is my habit to use `None` for a default value so that the underlying code can distinguish between "unspecified" and "explicitly false." But f that's counter to expectation here, I'm fine to change it.

…

On Sun, Feb 19, 2017, 9:44 AM Felix Fontein ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In nikola/nikola.py <#2674 (comment)>: > @@ -1649,7 +1649,7 @@ def apply_shortcodes(self, data, filename=None, lang=None, with_dependencies=Fal def generic_rss_renderer(self, lang, title, link, description, timeline, output_path, rss_teasers, rss_plain, feed_length=10, feed_url=None, - enclosure=_enclosure, rss_links_append_query=None): + enclosure=_enclosure, rss_links_append_query=None, itunes=None): Souldn't the default value for itunes be False and not None? After all, it is only used like a boolean. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2674 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVYVrrbUgbW6yKbDMlA6LI62SOsncVvks5reHFjgaJpZM4MFans> .

Kwpolska · 2017-02-19T16:52:36Z

None is good in this case IMO.

anderbubble · 2017-02-25T07:32:17Z

I see now that I've added explicit references to my "postcast" plugin in this Nikola core PR; so I'm going to have to factor that out.

Comments welcome on the other bits, though.

Kwpolska

Looks good, apart from minor style glitches.

Kwpolska · 2017-02-25T17:01:20Z

nikola/nikola.py

+                    or post.author(lang))
+                args['itunes_summary'] = post.meta[lang].get('itunes_summary') or data
+                if 'itunes_image' in post.meta[lang]:
+                    args['itunes_image'] = self.url_replacer(post.permalink(), post.meta[lang]['itunes_image'], lang, 'absolute')


Just keep in mind that the permalink won’t affect parsing of itunes_image paths, hey should be relative to site root (in your posts).

That hasn't been my experience. With this code, the image is located relative to the post's location, and then an absolute url to that location is generated.

edit: it appears to work exactly like a standard image directive.

Kwpolska · 2017-02-25T17:02:01Z

nikola/nikola.py

+                for tag in ('subtitle', 'duration', 'explicit'):
+                    key = 'itunes_{}'.format(tag)
+                    args[key] = post.meta[lang].get(key)
+                args['itunes_author'] = (


Write that on one line, will look better and fix a style checker complaint.

Kwpolska · 2017-02-25T17:02:42Z

nikola/utils.py

+        # It's an old style class
+        rss.RSS2.__init__(self, **kwargs)
+
+    def _itunes_attributes (self):


Remove the space before (self) here.

Oops. That's a minor stylistic affectation I use in my own code, but don't intend to inflict on anyone. Just a habit. :)

Kwpolska · 2017-02-25T17:04:00Z

nikola/utils.py


 class ExtendedItem(rss.RSSItem):
    """Extended RSS item."""

-    def __init__(self, **kw):
+    def __init__(self, itunes_author=None, itunes_subtitle=None,


It might be better to use the kw dict as well for your itunes attributes.

Doesn't that kind of reading-into-kwargs defeat static code analysis? If you don't declare what arguments you take in your function definition, you're unnecessarily hiding information.

At least, that's been my take on it in the past.

I would prefer to keep it this way if it was started this way — perhaps there’s a reason for using kw there?

anderbubble · 2017-02-27T03:19:23Z

I'm actually going a different way on this; leave this open for now, but I'd prefer to merge #2686 rather than this, and do my iTunes-specific stuff in my plugin rather than pollute core.

felixfontein · 2017-03-28T20:31:03Z

Since now #2686 is merged, can we close this @anderbubble?

anderbubble · 2017-04-24T19:27:11Z

@felixfontein for sure. Thanks.

Basic support for itunes rss tags

bca9baa

Kwpolska reviewed Feb 19, 2017

View reviewed changes

Kwpolska added the enhancement label Feb 19, 2017

Kwpolska reviewed Feb 19, 2017

View reviewed changes

anderbubble mentioned this pull request Feb 19, 2017

[WIP] Optionally read rss guid from the post #2675

Closed

felixfontein reviewed Feb 19, 2017

View reviewed changes

anderbubble changed the title ~~Basic support for itunes rss tags~~ [WIP] Basic support for itunes rss tags Feb 25, 2017

anderbubble mentioned this pull request Feb 25, 2017

Provide copyright field in RSS feed #2676

Merged

anderbubble added 4 commits February 25, 2017 03:24

Fix return from __init__

98291df

Refactor redundant code for itunes tag application

a3535c2

Better handling of itunes:image and cleanup

05e670c

Set itunes owner name

15c6ba2

Kwpolska reviewed Feb 25, 2017

View reviewed changes

anderbubble and others added 2 commits February 26, 2017 05:25

Stylistic cleanup

36c36a9

Remove trailing whitespace

7b15b44

anderbubble closed this Apr 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Basic support for itunes rss tags #2674

[WIP] Basic support for itunes rss tags #2674

anderbubble commented Feb 19, 2017 •

edited

Kwpolska Feb 19, 2017

Kwpolska commented Feb 19, 2017

Kwpolska Feb 19, 2017

anderbubble commented Feb 19, 2017 •

edited

Kwpolska commented Feb 19, 2017

felixfontein Feb 19, 2017

anderbubble commented Feb 19, 2017 via email

Kwpolska commented Feb 19, 2017

anderbubble commented Feb 25, 2017 •

edited

Kwpolska left a comment

Kwpolska Feb 25, 2017

anderbubble Feb 26, 2017 •

edited

Kwpolska Feb 25, 2017

Kwpolska Feb 25, 2017

anderbubble Feb 26, 2017

Kwpolska Feb 25, 2017

anderbubble Feb 26, 2017

Kwpolska Feb 26, 2017

anderbubble commented Feb 27, 2017

felixfontein commented Mar 28, 2017

anderbubble commented Apr 24, 2017

[WIP] Basic support for itunes rss tags #2674

[WIP] Basic support for itunes rss tags #2674

Conversation

anderbubble commented Feb 19, 2017 • edited

todo

Choose a reason for hiding this comment

Kwpolska commented Feb 19, 2017

Choose a reason for hiding this comment

anderbubble commented Feb 19, 2017 • edited

Kwpolska commented Feb 19, 2017

Choose a reason for hiding this comment

anderbubble commented Feb 19, 2017 via email

Kwpolska commented Feb 19, 2017

anderbubble commented Feb 25, 2017 • edited

Kwpolska left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anderbubble Feb 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anderbubble commented Feb 27, 2017

felixfontein commented Mar 28, 2017

anderbubble commented Apr 24, 2017

anderbubble commented Feb 19, 2017 •

edited

anderbubble commented Feb 19, 2017 •

edited

anderbubble commented Feb 25, 2017 •

edited

anderbubble Feb 26, 2017 •

edited