Refactor segmented file code for better modularity, testability and r…

…eusability

lacci/lib/shoes.rb

@@ -96,8 +96,8 @@ def run_app(relative_path)
       nil
     end
 
-    def file_loaders
-      @file_loaders ||= [
+    def default_file_loaders
+      [
         # By default we will always try to load any file, regardless of extension, as a Shoes Ruby file.
         proc do |path|
           load path
@@ -106,8 +106,20 @@ def file_loaders
       ]
     end
 
+    def file_loaders
+      @file_loaders ||= default_file_loaders
+    end
+
     def add_file_loader(loader)
       file_loaders.prepend(loader)
     end
+
+    def reset_file_loaders
+      @file_loaders = default_file_loaders
+    end
+
+    def set_file_loaders(loaders)
+      @file_loaders = loaders
+    end
   end
 end

lib/scarpe/wv.rb

@@ -22,7 +22,8 @@
 Shoes::Log.configure_logger(log_config)
 
 require "scarpe/components/segmented_file_loader"
-Shoes.add_file_loader Scarpe::Components::SEGMENTED_FILE_LOADER
+loader = Scarpe::Components::SegmentedFileLoader.new
+Shoes.add_file_loader loader
 
 require_relative "wv/web_wrangler"
 require_relative "wv/control_interface"

scarpe-components/lib/scarpe/components/file_helpers.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+require "tempfile"
+
+# These can be used for unit tests, but also more generally.
+
+module Scarpe::Components::FileHelpers
+  # Create a temporary file with the given prefix and contents.
+  # Execute the block of code with it in place. Make sure
+  # it gets cleaned up afterward.
+  #
+  # @param prefix [String] the prefix passed to Tempfile to identify this file on disk
+  # @param contents [String] the file contents that should be written to Tempfile
+  # @param dir [String] the directory to create the tempfile in
+  # @yield The code to execute with the tempfile present
+  # @yieldparam the path of the new tempfile
+  def with_tempfile(prefix, contents, dir: Dir.tmpdir)
+    t = Tempfile.new(prefix, dir)
+    t.write(contents)
+    t.flush # Make sure the contents are written out
+
+    yield(t.path)
+  ensure
+    t.close
+    t.unlink
+  end
+
+  # Create multiple tempfiles, with given contents, in given
+  # directories, and execute the block in that context.
+  # When the block is finished, make sure all tempfiles are
+  # deleted.
+  #
+  # Pass an array of arrays, where each array is of the form:
+  # [prefix, contents, (optional)dir]
+  #
+  # I don't love inlining with_tempfile's contents into here.
+  # But calling it iteratively or recursively was difficult
+  # when I tried it the obvious ways.
+  #
+  # This method should be equivalent to calling with_tempfile
+  # once for each entry in the array, in a set of nested
+  # blocks.
+  #
+  # @param tf_specs [Array<Array>] The array of tempfile prefixes, contents and directories
+  # @yield The code to execute with those tempfiles present
+  # @yieldparam An array of paths to tempfiles, in the same order as tf_specs
+  def with_tempfiles(tf_specs, &block)
+    tempfiles = []
+    tf_specs.each do |prefix, contents, dir|
+      dir ||= Dir.tmpdir
+      t = Tempfile.new(prefix, dir)
+      tempfiles << t
+      t.write(contents)
+      t.flush # Make sure the contents are written out
+    end
+
+    args = tempfiles.map(&:path)
+    yield(args)
+  ensure
+    tempfiles.each do |t|
+      t.close
+      t.unlink
+    end
+  end
+end

scarpe-components/lib/scarpe/components/segmented_file_loader.rb

@@ -1,93 +1,157 @@
 # frozen_string_literal: true
 
+require "scarpe/components/file_helpers"
+
 module Scarpe::Components
-  def self.segmented_file_load(path)
-    require "yaml" # Only load when needed
-    require "tempfile"
-    require "English"
+  class SegmentedFileLoader
+    include Scarpe::Components::FileHelpers
 
-    contents = File.read(path)
-    _front_matter = {}
+    # Add a new segment type (e.g. "catscradle") with a different
+    # file handler.
+    #
+    # @param type [String] the new name for this segment type
+    # @param handler [Object] an object that will be called as obj.call(filename) - often a proc
+    # @return <void>
+    def add_segment_type(type, handler)
+      if segment_type_hash.key?(type)
+        raise "Segment type #{type.inspect} already exists!"
+      end
 
-    segments = contents.split("\n-----")
+      segment_type_hash[type] = handler
+    end
 
-    if segments[0].start_with?("---/n") || segments[0] == "---"
-      # We have YAML front matter at the start.
-      # Eventually this will specify what different code segments do.
-      _front_matter = YAML.load segments[0]
-      if segments.size == 1
-        raise "Illegal segmented Scarpe file: must have at least one code segment, not just front matter!"
-      end
+    # Return an Array of segment type labels, such as "code" and "app_test".
+    #
+    # @return Array<String> the segment type labels
+    def segment_types
+      segment_type_hash.keys
+    end
+
+    # Load a .sca file with an optional YAML frontmatter prefix and
+    # multiple file sections which can be treated differently.
+    #
+    # The file loader acts like a proc, being called with .call()
+    # and returning true or false for whether it has handled the
+    # file load. This allows chaining loaders in order and the
+    # first loader to recognise a file will run it.
+    #
+    # @param path [String] the file or directory to treat as a Scarpe app
+    # @return [Boolean] return true if the file is loaded as a segmented Scarpe app file
+    def call(path)
+      return false unless path.end_with?(".scas")
 
-      segments = segments[1..-1]
-    elsif segments.size == 1
-      # Simplest segmented file there is: no front matter, no segments
-      # @todo: indicate to Scarpe in some way that this is a Scarpe-specific
-      #     file and extensions should be turned on?
-      return load path
+      file_load(path)
+      true
     end
 
-    # Beyond this point, our segmented file has at least one code segment with a five-plus-dashes divider
-    segmap = {}
-    segments.each do |segment|
-      if segment =~ /\A-*\s+(.*?)\n/
-        # named segment
-        segment = ::Regexp.last_match.post_match
-        segmap[::Regexp.last_match(1)] = segment
-      elsif segment[0] == "-"
-        # unnamed segment
-        segment =~ /\A-*/ || raise("Internal error! This regexp should always match!")
-        segment = ::Regexp.last_match.post_match
-        ctr = (1..10_000).detect { |i| !segmap.key?("%5d" % i) }
-        gen_name = "%5d" % ctr
-
-        segmap[gen_name] = segment
+    private
+
+    def gen_name(segmap)
+      ctr = (1..10_000).detect { |i| !segmap.key?("%5d" % i) }
+      "%5d" % ctr
+    end
+
+    def tokenize_segments(contents)
+      require "yaml" # Only load when needed
+      require "English"
+
+      segments = contents.split(/\n-{5,}/)
+      front_matter = {}
+
+      # The very first segment can start with front matter, or with a divider, or with no divider.
+      if segments[0].start_with?("---\n") || segments[0] == "---"
+        # We have YAML front matter at the start. All later segments will have a divider.
+        front_matter = YAML.load segments[0]
+        segments = segments[1..-1]
+      elsif segments[0].start_with?("-----")
+        # We have a divider at the start. Great! We're already well set up for this case.
+      elsif segments.size == 1
+        # No front matter, no divider, a single unnamed segment. No more parsing needed.
+        return [{}, { "" => segments[0] }]
       else
-        raise "Internal error when parsing segments in segmented app file!"
+        # No front matter, no divider before the first segment, multiple segments.
+        # We'll add an artificial divider to the first segment for uniformity.
+        segments = ["-----\n" + segments[0]] + segments[1..-1]
       end
-    end
 
-    if segmap.size == 1
-      # If there's only one segment, load it as Shoes code
-      eval segmap.values[0]
-    elsif segmap.size == 2
-      # If there are two segments, the first is Shoes code and the second is APP_TEST_CODE
-      segs = segmap.values
-      with_tempfile("scarpe_seg_test_code", segs[1]) do |tf|
-        # This will get picked up when Scarpe.app() runs. It will execute in the Scarpe::App.
-        # Note that unlike app_test_code in test_helper this does *not* load CatsCradle or
-        # set it up for testing.
-        ENV["SCARPE_APP_TEST"] = tf
-        eval segs[0]
+      segmap = {}
+      segments.each do |segment|
+        if segment =~ /\A-* +(.*?)\n/
+          # named segment with separator
+          segmap[::Regexp.last_match(1)] = ::Regexp.last_match.post_match
+        elsif segment =~ /\A-* *\n/
+          # unnamed segment with separator
+          segmap[gen_name(segmap)] = ::Regexp.last_match.post_match
+        else
+          raise "Internal error when parsing segments in segmented app file! seg: #{segment.inspect}"
+        end
       end
-    else
-      # Later there will be more interesting customisable options for what to do with
-      # different segments. For now, bail.
-      raise "More complex segmentation files are not yet implemented!"
+
+      [front_matter, segmap]
     end
-  end
 
-  # Can we share this with unit test helpers?
-  def self.with_tempfile(prefix, contents, dir: Dir.tmpdir)
-    t = Tempfile.new(prefix, dir)
-    t.write(contents)
-    t.flush # Make sure the contents are written out
+    def file_load(path)
+      contents = File.read(path)
 
-    yield(t.path)
-  ensure
-    t.close
-    t.unlink
-  end
+      front_matter, segmap = tokenize_segments(contents)
+
+      if segmap.empty?
+        raise "Illegal segmented Scarpe file: must have at least one code segment, not just front matter!"
+      end
+
+      # Match up front_matter[:segments] with the segments, or use the default of shoes and app_test.
+
+      if front_matter[:segments]
+        if front_matter[:segments].size != segmap.size
+          raise "Number of front matter :segments must equal number of file segments!"
+        end
+
+        sth = segment_type_hash
+        sv = segmap.values
+        front_matter[:segments].each.with_index do |seg_name, idx|
+          unless sth.key?(seg_name)
+            raise "Unrecognized segment type #{seg_name.inspect}! No matching segment type available!"
+          end
+
+          # Match the segment-type loader with the segment contents
+          with_tempfile("scarpe_segment_contents", sv[idx]) do |segment_contents_path|
+            sth[seg_name].call(segment_contents_path)
+          end
+        end
+      elsif segmap.size == 1
+        # If there's only one segment, load it as Shoes code
+        eval segmap.values[0]
+      elsif segmap.size == 2
+        # If there are two segments, the first is Shoes code and the second is APP_TEST_CODE
+        # TODO: write a better loader for app_test code that doesn't set an env var and generally
+        #    break ordering.
+        segs = segmap.values
+        with_tempfile("scarpe_seg_test_code", segs[1]) do |tf|
+          # This will get picked up when Scarpe.app() runs. It will execute in the Scarpe::App.
+          # Note that unlike app_test_code in test_helper this does *not* load CatsCradle or
+          # set it up for testing.
+          ENV["SCARPE_APP_TEST"] = tf
+          eval segs[0]
+        end
+      else
+        raise "Segmented files with more than two segments have to specify what they're for!"
+      end
+    end
 
-  # Load a .sca file with an optional YAML frontmatter prefix and
-  # multiple file sections which can be treated differently.
-  SEGMENTED_FILE_LOADER = lambda do |path|
-    if path.end_with?(".scas")
-      Scarpe::Components.segmented_file_load(path)
-      return true
+    # The hash of segment type labels mapped to handlers which will be called.
+    # Normal client code shouldn't ever call this.
+    #
+    # @return Hash<String, Object> the name/handler pairs
+    def segment_type_hash
+      @segment_handlers ||= {
+        "shoes" => proc { |seg_file| load seg_file },
+        "app_test" => proc { |seg_file| ENV["SCARPE_APP_TEST"] = seg_file },
+      }
     end
-    false
   end
 end
 
-# Shoes.add_file_loader Scarpe::Components::SEGMENTED_FILE_LOADER
+# You can add additional segment types to the segmented file loader
+# loader = Scarpe::Components::SegmentedFileLoader.new
+# loader.add_segment_type "capybara", proc { |seg_file| load_file_as_capybara(seg_file) }
+# Shoes.add_file_loader loader