spring cleaning util and discovery of nasty variable mutating in url_for

middleman-core/lib/middleman-core/application.rb

@@ -16,6 +16,9 @@ require 'active_support/core_ext/float/rounding'
 # Simple callback library
 require 'vendored-middleman-deps/hooks-0.2.0/lib/hooks'
 
+# Our custom logger
+require 'middleman-core/logger'
+
 require 'middleman-core/sitemap'
 
 require 'middleman-core/configuration'
@@ -156,8 +159,12 @@ module Middleman
     # with_layout and page routing
     include Middleman::CoreExtensions::Routing
 
+    attr_reader :logger
+
     # Initialize the Middleman project
     def initialize(&block)
+      @logger = ::Middleman::Logger.singleton
+
       # Clear the static class cache
       cache.clear
 
@@ -188,11 +195,15 @@ module Middleman
 
     # Whether we're in development mode
     # @return [Boolean] If we're in dev mode
-    def development?; config[:environment] == :development; end
+    def development?
+      config[:environment] == :development
+    end
 
     # Whether we're in build mode
     # @return [Boolean] If we're in build mode
-    def build?; config[:environment] == :build; end
+    def build?
+      config[:environment] == :build
+    end
 
     # The full path to the source directory
     #
@@ -201,7 +212,7 @@ module Middleman
       File.join(root, config[:source])
     end
 
-    delegate :logger, :instrument, :to => ::Middleman::Util
+    delegate :instrument, :to => ::Middleman::Util
 
     # Work around this bug: http://bugs.ruby-lang.org/issues/4521
     # where Ruby will call to_s/inspect while printing exception
@@ -212,27 +223,6 @@ module Middleman
     end
     alias :inspect :to_s # Ruby 2.0 calls inspect for NoMethodError instead of to_s
 
-    # Expand a path to include the index file if it's a directory
-    #
-    # @private
-    # @param [String] path Request path
-    # @return [String] Path with index file if necessary
-    def full_path(path)
-      resource = sitemap.find_resource_by_destination_path(path)
-
-      if !resource
-        # Try it with /index.html at the end
-        indexed_path = File.join(path.sub(%r{/$}, ''), config[:index_file])
-        resource = sitemap.find_resource_by_destination_path(indexed_path)
-      end
-
-      if resource
-        '/' + resource.destination_path
-      else
-        '/' + Middleman::Util.normalize_path(path)
-      end
-    end
-
   end
 end
 

middleman-core/lib/middleman-core/cli/build.rb

@@ -1,4 +1,6 @@
 require 'middleman-core'
+require 'middleman-core/logger'
+
 require 'fileutils'
 require 'set'
 
@@ -90,7 +92,7 @@ module Middleman::Cli
       def shared_instance(verbose=false, instrument=false)
         @_shared_instance ||= ::Middleman::Application.server.inst do
           config[:environment] = :build
-          logger(verbose ? 0 : 1, instrument)
+          ::Middleman::Logger.singleton(verbose ? 0 : 1, instrument)
         end
       end
     end

middleman-core/lib/middleman-core/core_extensions/request.rb

@@ -4,6 +4,8 @@ require 'rack/file'
 require 'rack/lint'
 require 'rack/head'
 
+require 'middleman-core/util'
+
 module Middleman
   module CoreExtensions
 
@@ -231,7 +233,7 @@ module Middleman
           if request_path.respond_to? :force_encoding
             request_path.force_encoding('UTF-8')
           end
-          request_path = full_path(request_path)
+          request_path = ::Middleman::Util.full_path(request_path, self)
 
           # Run before callbacks
           run_hook :before

middleman-core/lib/middleman-core/logger.rb

@@ -7,6 +7,18 @@ module Middleman
 
   # The Middleman Logger
   class Logger < ActiveSupport::BufferedLogger
+
+    def self.singleton(*args)
+      if !@_logger || args.length > 0
+        if args.length == 1 && (args.first.is_a?(::String) || args.first.respond_to?(:write))
+          args = [0, false, args.first]
+        end
+        @_logger = new(*args)
+      end
+
+      @_logger
+    end
+
     def initialize(log_level=1, is_instrumenting=false, target=$stdout)
       super(target)
 

middleman-core/lib/middleman-core/preview_server.rb

@@ -1,5 +1,6 @@
 require 'webrick'
 require 'middleman-core/meta_pages'
+require 'middleman-core/logger'
 
 module Middleman
   module PreviewServer
@@ -90,7 +91,7 @@ module Middleman
 
     private
       def new_app
-        opts = @options
+        opts = @options.dup
         server = ::Middleman::Application.server
 
         # Add in the meta pages application
@@ -100,11 +101,14 @@ module Middleman
         end
 
         @app = server.inst do
+          ::Middleman::Logger.singleton(
+            opts[:debug] ? 0 : 1,
+            opts[:instrumenting] || false
+          )
+
           if opts[:environment]
             config[:environment] = opts[:environment].to_sym
           end
-
-          logger(opts[:debug] ? 0 : 1, opts[:instrumenting] || false)
         end
       end
 

middleman-core/lib/middleman-core/sitemap/extensions/redirects.rb

@@ -78,7 +78,10 @@ module Middleman
           end
 
           def render(*args, &block)
-            url = ::Middleman::Util.url_for(store.app, @request_path, :relative => false, :find_resource => true)
+            url = ::Middleman::Util.url_for(store.app, @request_path, {
+              :relative => false,
+              :find_resource => true
+            })
 
             if output
               output.call(path, url)

middleman-core/lib/middleman-core/sitemap/store.rb

@@ -252,10 +252,10 @@ module Middleman
       # @param [String] path
       # @return [String]
       def strip_away_locale(path)
-        if app.respond_to? :langs
+        if @app.respond_to? :langs
           path_bits = path.split('.')
           lang = path_bits.last
-          if app.langs.include?(lang.to_sym)
+          if @app.langs.include?(lang.to_sym)
             return path_bits[0..-2].join('.')
           end
         end

middleman-core/lib/middleman-core/util.rb

@@ -1,6 +1,3 @@
-# Our custom logger
-require 'middleman-core/logger'
-
 # For instrumenting
 require 'active_support/notifications'
 
@@ -10,57 +7,39 @@ require 'thor'
 # Core Pathname library used for traversal
 require 'pathname'
 
+# Template and Mime detection
 require 'tilt'
 require 'rack/mime'
 
 module Middleman
-
   module Util
+    class << self
 
       # Whether the source file is binary.
       #
       # @param [String] filename The file to check.
       # @return [Boolean]
-    def self.binary?(filename)
+      def binary?(filename)
         ext = File.extname(filename)
+
+        # We hardcode detecting of gzipped SVG files
         return true if ext == '.svgz'
-      return false if Tilt.registered?(ext.sub('.',''))
 
-      ext = ".#{ext}" unless ext.to_s[0] == ?.
+        return false if Tilt.registered?(ext.sub('.', ''))
-      mime = ::Rack::Mime.mime_type(ext, nil)
-      unless mime
-        binary_bytes = [0, 1, 2, 3, 4, 5, 6, 11, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 28, 29, 30, 31]
-        s = File.read(filename, 4096) || ''
-        s.each_byte do |c|
-          return true if binary_bytes.include?(c)
-        end
-        return false
-      end
-      return false if mime.start_with?('text/')
-      return false if mime.include?('xml')
-      return false if mime.include?('json')
-      return false if mime.include?('javascript')
-      true
-    end
 
-    # The logger
+        dot_ext = (ext.to_s[0] == ?.) ? ext.dup : ".#{ext}"
-    #
-    # @return [Middleman::Logger] The logger
-    def self.logger(*args)
-      if !@_logger || args.length > 0
-        if args.length == 1 && (args.first.is_a?(::String) || args.first.respond_to?(:write))
-          args = [0, false, args.first]
-        end
-        @_logger = ::Middleman::Logger.new(*args)
-      end
         
-      @_logger
+        if mime = ::Rack::Mime.mime_type(dot_ext, nil)
+          !nonbinary_mime?(mime)
+        else
+          file_contents_include_binary_bytes?(filename)
+        end
       end
 
       # Facade for ActiveSupport/Notification
-    def self.instrument(name, payload={}, &block)
+      def instrument(name, payload={}, &block)
-      name << '.middleman' unless name =~ /\.middleman$/
+        suffixed_name = (name =~ /\.middleman$/) ? name.dup : "#{name}.middleman"
-      ::ActiveSupport::Notifications.instrument(name, payload, &block)
+        ::ActiveSupport::Notifications.instrument(suffixed_name, payload, &block)
       end
 
       # Recursively convert a normal Hash into a HashWithIndifferentAccess
@@ -68,7 +47,7 @@ module Middleman
       # @private
       # @param [Hash] data Normal hash
       # @return [Thor::CoreExt::HashWithIndifferentAccess]
-    def self.recursively_enhance(data)
+      def recursively_enhance(data)
         if data.is_a? Hash
           data = ::Thor::CoreExt::HashWithIndifferentAccess.new(data)
           data.each do |key, val|
@@ -88,14 +67,14 @@ module Middleman
       # Normalize a path to not include a leading slash
       # @param [String] path
       # @return [String]
-    def self.normalize_path(path)
+      def normalize_path(path)
         # The tr call works around a bug in Ruby's Unicode handling
         path.sub(%r{^/}, '').tr('','')
       end
 
       # This is a separate method from normalize_path in case we
       # change how we normalize paths
-    def self.strip_leading_slash(path)
+      def strip_leading_slash(path)
         path.sub(%r{^/}, '')
       end
 
@@ -103,7 +82,7 @@ module Middleman
       # Useful for extensions implemented as Rack middleware.
       # @param response The response from #call
       # @return [String] The whole response as a string.
-    def self.extract_response_text(response)
+      def extract_response_text(response)
         # The rack spec states all response bodies must respond to each
         result = ''
         response.each do |part, s|
@@ -121,13 +100,14 @@ module Middleman
       # @param matcher A matcher string/regexp/proc/etc
       # @param path A path as a string
       # @return [Boolean] Whether the path matches the matcher
-    def self.path_match(matcher, path)
+      def path_match(matcher, path)
-      if matcher.is_a? String
+        case
-        path.match matcher
+        when matcher.is_a?(String)
-      elsif matcher.respond_to? :match
+          path.match(matcher)
-        matcher.match path
+        when matcher.respond_to?(:match)
-      elsif matcher.respond_to? :call
+          matcher.match(path)
-        matcher.call path
+        when matcher.respond_to?(:call)
+          matcher.call(path)
         else
           File.fnmatch(matcher.to_s, path)
         end
@@ -138,9 +118,10 @@ module Middleman
       #
       # @param paths Some paths string or Pathname
       # @return [Array] An array of filenames
-    def self.all_files_under(*paths)
+      def all_files_under(*paths)
         paths.flat_map do |p|
           path = Pathname(p)
+
           if path.directory?
             all_files_under(*path.children)
           elsif path.file?
@@ -152,11 +133,15 @@ module Middleman
       # Given a source path (referenced either absolutely or relatively)
       # or a Resource, this will produce the nice URL configured for that
       # path, respecting :relative_links, directory indexes, etc.
-    def self.url_for(app, path_or_resource, options={})
+      def url_for(app, path_or_resource, options={})
         # Handle Resources and other things which define their own url method
-      url = path_or_resource.respond_to?(:url) ? path_or_resource.url : path_or_resource
+        url = if path_or_resource.respond_to?(:url)
-      url = url.gsub(' ', '%20')
+          path_or_resource.url
+        else
+          path_or_resource.dup
+        end.gsub(' ', '%20')
 
+        # Try to parse URL
         begin
           uri = URI(url)
         rescue URI::InvalidURIError
@@ -164,7 +149,7 @@ module Middleman
           return url
         end
 
-      relative = options.delete(:relative)
+        relative = options[:relative]
         raise "Can't use the relative option with an external URL" if relative && uri.host
 
         # Allow people to turn on relative paths for all links with
@@ -174,7 +159,7 @@ module Middleman
         effective_relative = true if relative.nil? && app.config[:relative_links]
 
         # Try to find a sitemap resource corresponding to the desired path
-      this_resource = app.current_resource # store in a local var to save work
+        this_resource = options[:current_resource]
 
         if path_or_resource.is_a?(::Middleman::Sitemap::Resource)
           resource = path_or_resource
@@ -192,11 +177,93 @@ module Middleman
         end
 
         if resource
-        # Switch to the relative path between this_resource and the given resource
+          uri.path = relative_path_from_resource(this_resource, resource_url, effective_relative)
+        else
+          # If they explicitly asked for relative links but we can't find a resource...
+          raise "No resource exists at #{url}" if relative
+        end
+
+        # Support a :query option that can be a string or hash
+        if query = options[:query]
+          uri.query = query.respond_to?(:to_param) ? query.to_param : query.to_s
+        end
+
+        # Support a :fragment or :anchor option just like Padrino
+        fragment = options[:anchor] || options[:fragment]
+        uri.fragment = fragment.to_s if fragment
+
+        # Finally make the URL back into a string
+        uri.to_s
+      end
+
+      # Expand a path to include the index file if it's a directory
+      #
+      # @param [String] path Request path/
+      # @param [Middleman::Application] app The requesting app.
+      # @return [String] Path with index file if necessary.
+      def full_path(path, app)
+        resource = app.sitemap.find_resource_by_destination_path(path)
+
+        if !resource
+          # Try it with /index.html at the end
+          indexed_path = File.join(path.sub(%r{/$}, ''), app.config[:index_file])
+          resource = app.sitemap.find_resource_by_destination_path(indexed_path)
+        end
+
+        if resource
+          '/' + resource.destination_path
+        else
+          '/' + normalize_path(path)
+        end
+      end
+
+    private
+
+      # Is mime type known to be non-binary?
+      #
+      # @param [String] mime The mimetype to check.
+      # @return [Boolean]
+      def nonbinary_mime?(mime)
+        case
+        when mime.start_with?('text/')
+          true
+        when mime.include?('xml')
+          true
+        when mime.include?('json')
+          true
+        when mime.include?('javascript')
+          true
+        else
+          false
+        end
+      end
+
+      # Read a few bytes from the file and see if they are binary.
+      #
+      # @param [String] filename The file to check.
+      # @return [Boolean]
+      def file_contents_include_binary_bytes?(filename)
+        binary_bytes = [0, 1, 2, 3, 4, 5, 6, 11, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 28, 29, 30, 31]
+        s = File.read(filename, 4096) || ''
+        s.each_byte do |c|
+          return true if binary_bytes.include?(c)
+        end
+        
+        false
+      end
+
+      # Get a relative path to a resource.
+      #
+      # @param [Middleman::Sitemap::Resource] curr_resource The resource.
+      # @param [String] resource_url The target url.
+      # @param [Boolean] relative If the path should be relative.
+      # @return [String]
+      def relative_path_from_resource(curr_resource, resource_url, relative)
+        # Switch to the relative path between resource and the given resource
         # if we've been asked to.
-        if effective_relative
+        if relative && curr_resource
           # Output urls relative to the destination path, not the source path
-          current_dir = Pathname('/' + this_resource.destination_path).dirname
+          current_dir = Pathname('/' + curr_resource.destination_path).dirname
           relative_path = Pathname(resource_url).relative_path_from(current_dir).to_s
 
           # Put back the trailing slash to avoid unnecessary Apache redirects
@@ -204,26 +271,11 @@ module Middleman
             relative_path << '/'
           end
 
-          uri.path = relative_path
+          relative_path
         else
-          uri.path = resource_url
+          resource_url
         end
-      else
-        # If they explicitly asked for relative links but we can't find a resource...
-        raise "No resource exists at #{url}" if relative
       end
-
-      # Support a :query option that can be a string or hash
-      if query = options.delete(:query)
-        uri.query = query.respond_to?(:to_param) ? query.to_param : query.to_s
-      end
-
-      # Support a :fragment or :anchor option just like Padrino
-      fragment = options.delete(:anchor) || options.delete(:fragment)
-      uri.fragment = fragment.to_s if fragment
-
-      # Finally make the URL back into a string
-      uri.to_s
     end
   end
 end

middleman-core/lib/middleman-more/core_extensions/default_helpers.rb

@@ -194,7 +194,10 @@ class Middleman::CoreExtensions::DefaultHelpers < ::Middleman::Extension
     # or a Resource, this will produce the nice URL configured for that
     # path, respecting :relative_links, directory indexes, etc.
     def url_for(path_or_resource, options={})
-      ::Middleman::Util.url_for(self, path_or_resource, options)
+      options_with_resource = options.dup
+      options_with_resource[:current_resource] ||= current_resource
+
+      ::Middleman::Util.url_for(self, path_or_resource, options_with_resource)
     end
 
     # Overload the regular link_to to be sitemap-aware - if you
@@ -224,6 +227,14 @@ class Middleman::CoreExtensions::DefaultHelpers < ::Middleman::Extension
 
         # Transform the url through our magic url_for method
         args[url_arg_index] = url_for(url, options)
+
+        # Cleanup before passing to Padrino
+        options.delete(:relative)
+        options.delete(:current_resource)
+        options.delete(:find_resource)
+        options.delete(:query)
+        options.delete(:anchor)
+        options.delete(:fragment)
       end
 
       super(*args, &block)

middleman-core/lib/middleman-more/extensions/asset_hash.rb

@@ -1,3 +1,5 @@
+require 'middleman-core/util'
+
 class Middleman::Extensions::AssetHash < ::Middleman::Extension
   option :exts, %w(.jpg .jpeg .png .gif .js .css .otf .woff .eot .ttf .svg), 'List of extensions that get asset hashes appended to them.'
   option :ignore, [], 'Regexes of filenames to skip adding asset hashes to'
@@ -70,7 +72,7 @@ class Middleman::Extensions::AssetHash < ::Middleman::Extension
       # We don't want to use this middleware when rendering files to figure out their hash!
       return [status, headers, response] if env['bypass_asset_hash'] == 'true'
 
-      path = @middleman_app.full_path(env['PATH_INFO'])
+      path = ::Middleman::Util.full_path(env['PATH_INFO'], @middleman_app)
 
       if path =~ /(^\/$)|(\.(htm|html|php|css|js)$)/
         body = ::Middleman::Util.extract_response_text(response)