Fluid tag for displaying GitHub Gists with noscript tags in a Jekyll post A while back I wrote a Fluid tag for Jekyll that inserts the contents of a GitHub Gist into the content of the page in &lt;noscript&gt; tags for RSS readers. I've noticed a couple things I'm not happy with since posting it, but I am interested in the critique it will generate here on Stack Exchange!

(Obviously, some of the internals regarding methods required to make Liquid tags work (such as the parameters to initialize and having a render method) aren't negotiable, but you get the idea.)

require 'digest/md5'
require 'net/https'
require 'uri'

module Jekyll
  class GistTag &lt; Liquid::Tag
    def initialize(tag_name, text, token)
      super
      @text         = text
      @cache        = true
      @cache_folder = File.expand_path "../_gist_cache", File.dirname(__FILE__)
    end

    def render(context)
      return "" unless @text =~ /([\d]*) (.*)/

      gist, file = $1.strip, $2.strip
      script_url = "https://gist.github.com/#{gist}.js?file=#{file}"

      code       = get_cached_gist(gist, file) || get_gist_from_web(gist, file)
      code       = code.gsub "&lt;", "&amp;lt;"
      string     = "&lt;script src='#{script_url}'&gt;&lt;/script&gt;"
      string    += "&lt;noscript&gt;&lt;pre&gt;&lt;code&gt;#{code}&lt;/code&gt;&lt;/pre&gt;&lt;/noscript&gt;"
      return string
    end

    def get_gist_url_for(gist, file)
      "https://gist.github.com/raw/#{gist}/#{file}"
    end

    def cache_gist(gist, file, data)
      file = get_cache_file_for gist, file
      File.open(file, "w+") do |f|
        f.write(data)
      end
    end

    def get_cached_gist(gist, file)
      return nil if @cache == false
      file = get_cache_file_for gist, file
      return nil unless File.exist?(file)
      return File.new(file).read
    end

    def get_cache_file_for(gist, file)
      bad_chars = /[^a-zA-Z0-9\-_\.]/
      gist      = gist.gsub bad_chars, ''
      file      = file.gsub bad_chars, ''
      md5       = Digest::MD5.hexdigest "#{gist}-#{file}"
      File.join @cache_folder, "#{gist}-#{file}-#{md5}.cache"
    end

    def get_gist_from_web(gist, file)
      gist_url          = get_gist_url_for(gist, file)
      raw_uri           = URI.parse(gist_url)
      https             = Net::HTTP.new(raw_uri.host, raw_uri.port)
      https.use_ssl     = true
      https.verify_mode = OpenSSL::SSL::VERIFY_NONE
      request           = Net::HTTP::Get.new(raw_uri.request_uri)
      data              = https.request(request)
      data              = data.body
      cache_gist(gist, file, data) unless @cache == false
      data
    end
  end

  class GistTagNoCache &lt; GistTag
    def initialize(tag_name, text, token)
      super
      @cache = false
    end
  end
end

Liquid::Template.register_tag('gist', Jekyll::GistTag)
Liquid::Template.register_tag('gistnocache', Jekyll::GistTagNoCache)
A couple of mostly minor notes and improvements:

return "" unless @text =~ /([\d]*) (.*)/
gist, file = $1.strip, $2.strip


I generally try to avoid magic variables where ever possible. Using match and the resulting MatchData object tends to be less error prone.



code       = code.gsub "&lt;", "&amp;lt;"


Is there a reason you only escape &lt; characters? Unless I'm missing something, it seems preferable to use CGI.escapeHTML instead.



return string


It seems inconsistent to me that in some methods you have a return statement on the last line and in others you do not. I would always leave out the return statement on the last line as it is unnecessary.



return nil if @cache == false


It is pretty unidiomatic to use == with boolean literals - usually one would just negate the boolean instead of comparing to false. if !@cache or better unless @cache will read more natural to most people than if @cache == false.



File.new(file).read


You should never use File.new without storing its return value somewhere and then later calling close on it. As a matter of fact in most cases you should not use File.new at all, but rather File.open with a block, which closes the file automatically when the block's end is reached. The way you're doing it the file handle will remain open until the File object is garbage collected, which may be a while. This is a bad practice and can lead to subtle problems.

However in this case you don't even need File.open as there is already a method which does exactly what you need: File.read(file).



gist.gsub! /[^a-zA-Z0-9\-_\.]/, ''


You don't need to escape . as it has no special meaning inside brackets.



cache_gist(gist, file, data) unless @cache == false


This would really be a lot more readable without the explicit comparison and the double negation:

cache_gist(gist, file, data) if @cache