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 support for Podcast RSS feed imports #413

Merged
merged 10 commits into from
Apr 24, 2021
Merged

Conversation

nicktmro
Copy link
Contributor

@nicktmro nicktmro commented May 31, 2019

This is very similar to the existing RSS importer. Unlike the RSS importer, this importer will also extract the enclosure tag and wrap the url into an audio HTML 5 tag so that compliant browsers can just play the audio file.

Standard "shownotes" can be extracted from a feed by specifying the body launch argument as per the included documentation.

There is also support to not overwrite previous files using the overwrite launch argument with a false value. The default is to overwrite any existing files.

Nick Parfene added 4 commits May 26, 2019 14:46
The importer will generate posts that use the podcast episode timestamp, the notes, as well as the audio file.

For now, use open-embed to auto-magically turn the audio file into a playable resource via the html 5 audio tag..
Default overwrite remains true but it's now possible to toggle to false and maintain the edits in the _posts folder

The audio file is now wrapped in an HTML audio tag so that browser can play it natively.
@DirtyF DirtyF added the feature label May 31, 2019
@DirtyF DirtyF requested a review from a team May 31, 2019 05:51
Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this new importer, please remove the example post here.

@nicktmro
Copy link
Contributor Author

Done

Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned in the comment #412 (comment), please subclass the existing importer to avoid repeating identical method definitions:

module JekyllImport
  module Importers
    class RSSPodcast < RSS
      # new definitions only
    end
  end
end

@nicktmro
Copy link
Contributor Author

@ashmaroli I tried extending the RSS Importer (as per your instructions) and leaving just the def self.process(options), since it's where all my changes went, but I get this:

rss_podcast.rb:5:in `<module:Importers>': uninitialized constant JekyllImport::Importers::RSS (NameError)

As I mentioned in #412 I am not a Ruby dev. My attempts to import / reference the RSS.rb file have all failed, and when I checked all the other Importers, I noticed that none of them actually extend anything other than Importer so I coudn't follow a pattern. My googling is also taking me down rabbit holes and I was wondering if you could shed more light on how to solve the error above?

@ashmaroli
Copy link
Member

@nicktmro Perhaps the error you're seeing is because the rss_podcast.rb is being run before rss.rb has been loaded into memory.
So lets try an ensure that the rss.rb has been loaded beforehand by adding a call to require_relative at the top of the rss_podcast.rb file:

# frozen_string_literal: true

require_relative "rss"

module JekyllImport
  module Importers
    class RSSPodcast < RSS
      # new definitions
    end
  end
end

If the above works, do make the change.
Otherwise, let me know and I shall remove the request to make changes to this PR.
Thank you.

@nicktmro
Copy link
Contributor Author

That did the trick :)

@ashmaroli
Copy link
Member

The sole changes in this importer as compared to the RSS Importer is actually:

@@ -30,10 +30,11 @@ module JekyllImport
       # Returns nothing.
       def self.process(options)
         source = options.fetch("source")
         frontmatter = options.fetch("frontmatter", [])
         body = options.fetch("body", ["description"])
+        overwrite = options.fetch("overwrite", true)
 
         content = ""
         open(source) { |s| content = s.read }
         rss = ::RSS::Parser.parse(content, false)
 
@@ -42,10 +43,15 @@ module JekyllImport
         rss.items.each do |item|
           formatted_date = item.date.strftime("%Y-%m-%d")
           post_name = Jekyll::Utils.slugify(item.title, :mode => "latin")
           name = "#{formatted_date}-#{post_name}"
 
+          # Skip this file if it already exists and overwrite is turned off
+          next if !overwrite && File.file?("_posts/#{name}.html")
+
+          audio = item.enclosure.url
+
           header = {
             "layout" => "post",
             "title"  => item.title,
           }
 
@@ -67,10 +73,11 @@ module JekyllImport
           FileUtils.mkdir_p("_posts")
 
           File.open("_posts/#{name}.html", "w") do |f|
             f.puts header.to_yaml
             f.puts "---\n\n"
+            f.puts "<audio controls=\"\"><source src=\"#{audio}\" type=\"audio/mpeg\">Your browser does not support the audio element.</audio>"
             f.puts output
           end
         end
       end
     end

The rest are identical.. Perhaps we should just "enhance the existing RSS importer" with some options instead of extending it with a few minor changes.. 🤔

@nicktmro
Copy link
Contributor Author

That sounds good to me, but I probably lack the skill to make that happen. Happy to help test it or justify the current “options”, though.

@ashmaroli
Copy link
Member

@nicktmro I've merged your rss_podcast importer into the existing rss importer with a few changes. (Notably, I removed the overwrite configuration since I felt that to be outside the scope of this PR's primary intention.)
However, feel free to make changes to suit your original use-case and yeah, update the documentation once the implementation is stable.

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with @ashmaroli implementation of the ability to render audio elements.

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Could you please update the documentation for the RSS reader?

@ashmaroli ashmaroli dismissed their stale review November 4, 2019 17:13

Concerns have been addressed..

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@parkr
Copy link
Member

parkr commented Apr 24, 2021

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit f9f4c2f into jekyll:master Apr 24, 2021
jekyllbot added a commit that referenced this pull request Apr 24, 2021
@jekyll jekyll locked and limited conversation to collaborators Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants