Get rid of Middleman::Sitemap::Store#all_paths and #each in favor of #pages. Dealing with page objects all over the place instead of paths provides a lot of opportunities for cleaner code.

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

@@ -197,10 +197,11 @@ module Middleman::Cli
       # Sort order, images, fonts, js/css and finally everything else.
       sort_order = %w(.png .jpeg .jpg .gif .bmp .svg .svgz .ico .woff .otf .ttf .eot .js .css)
       
-      @app.sitemap.all_paths.select do |p|
+      # Pre-request CSS to give Compass a chance to build sprites
-        File.extname(p) == ".css"
+      @app.sitemap.pages.select do |p|
+        p.ext == ".css"
       end.each do |p|
-        Middleman::Cli::Build.shared_rack.get("/" + p.gsub(/\s/, "%20"))
+        Middleman::Cli::Build.shared_rack.get(p.request_path.gsub(/\s/, "%20"))
       end
       
       # Double-check for compass sprites
@@ -210,23 +211,17 @@ module Middleman::Cli
       # find files in the build folder when it needs to generate sprites for the
       # css files
 
-      # TODO: deal with pages, not paths
+      pages = @app.sitemap.pages.sort do |a, b|
-      paths = @app.sitemap.all_paths.sort do |a, b|
+        a_idx = sort_order.index(a.ext) || 100
-        a_ext = File.extname(a)
+        b_idx = sort_order.index(b.ext) || 100
-        b_ext = File.extname(b)
-
-        a_idx = sort_order.index(a_ext) || 100
-        b_idx = sort_order.index(b_ext) || 100
 
         a_idx <=> b_idx
       end
 
       # Loop over all the paths and build them.
-      paths.each do |path|
+      pages.each do |page|
-        page = @app.sitemap.page(path)
-
         next if page.ignored?
-        next if @config[:glob] && !File.fnmatch(@config[:glob], path)
+        next if @config[:glob] && !File.fnmatch(@config[:glob], page.path)
 
         base.tilt_template(page)
 

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

@@ -72,7 +72,7 @@ module Middleman::Sitemap
       @_template ||= ::Middleman::Sitemap::Template.new(self)
     end
     
-    # Extension of the path
+    # Extension of the path (i.e. '.js')
     # @return [String]
     def ext
       File.extname(path)
@@ -235,29 +235,27 @@ module Middleman::Sitemap
 
       if eponymous_directory?
         base_path = eponymous_directory_path
-        prefix    = /^#{base_path.sub("/", "\\/")}/
+        prefix    = %r|^#{base_path.sub("/", "\\/")}|
       else
         base_path = path.sub("#{app.index_file}", "")
-        prefix    = /^#{base_path.sub("/", "\\/")}/
+        prefix    = %r|^#{base_path.sub("/", "\\/")}|
       end
             
-      store.all_paths.select do |sub_path|
+      store.pages.select do |sub_page|
-        sub_path =~ prefix
+        if sub_page == self || sub_page.path !~ prefix || sub_page.ignored?
-      end.select do |sub_path|
+          false
-        path != sub_path
+        else
-      end.select do |sub_path|
+          inner_path = sub_page.path.sub(prefix, "")
-       inner_path = sub_path.sub(prefix, "")
+          parts = inner_path.split("/")
-       parts = inner_path.split("/")
+          if parts.length == 1
-       if parts.length == 1
+            true
-         true
+          elsif parts.length == 2
-       elsif parts.length == 2
+            parts.last == app.index_file
-         parts.last == app.index_file
+          else
-       else
+            false
-         false
+          end
-       end
+        end
-      end.map do |p| 
+      end
-        store.page(p)
-      end.reject { |p| p.ignored? }
     end
     
     # This page's sibling pages

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

@@ -24,6 +24,12 @@ module Middleman::Sitemap
       @reroute_callbacks   = []
     end
     
+    # A list of all pages
+    # @return [Array<Middleman::Sitemap::Page>]
+    def pages
+      @pages.values
+    end
+
     # Check to see if we know about a specific path
     # @param [String] path
     # @return [Boolean]
@@ -82,22 +88,7 @@ module Middleman::Sitemap
     def page_by_destination(destination_path)
       # TODO: memoize this
       destination_path = normalize_path(destination_path)
-      @pages.values.find {|p| p.destination_path == destination_path }
+      pages.find {|p| p.destination_path == destination_path }
-    end
-    
-    # Loop over known pages
-    # @yield [path, page]
-    # @return [void]
-    def each
-      @pages.each do |k, v|
-        yield k, v
-      end
-    end
-
-    # Get all known paths
-    # @return [Array<String>]
-    def all_paths
-      @pages.keys
     end
     
     # Whether a path is ignored
@@ -119,7 +110,7 @@ module Middleman::Sitemap
     # Get a list of ignored paths
     # @return [Array<String>]
     def ignored_paths
-      @pages.values.select(&:ignored?).map(&:path)
+      pages.select(&:ignored?).map(&:path)
     end
     
     # Whether the given path is generic
@@ -133,7 +124,7 @@ module Middleman::Sitemap
     # @return [Array<String>]
     def generic_paths
       app.cache.fetch :generic_paths do
-        @pages.values.select(&:generic?).map(&:path)
+        pages.select(&:generic?).map(&:path)
       end
     end
     
@@ -148,7 +139,7 @@ module Middleman::Sitemap
     # @return [Array<String>]
     def proxied_paths
       app.cache.fetch :proxied_paths do
-        @pages.values.select(&:proxy?).map(&:path)
+        pages.select(&:proxy?).map(&:path)
       end
     end