Asciidoctor: Copy referenced images

lib/ES/Util.pm

@@ -79,8 +79,6 @@ sub build_chunked {
             $output = run(
                 'asciidoctor', '-v', '--trace',
                 '-r' => dir('resources/asciidoctor/lib/extensions.rb')->absolute,
-                # TODO figure out resource
-                # ( map { ( '--resource' => $_ ) } @$resources ),
                 '-b' => 'docbook45',
                 '-d' => 'book',
                 '-a' => 'showcomments=1',
@@ -94,6 +92,7 @@ sub build_chunked {
                 # missing attributes!
                 # '-a' => 'attribute-missing=warn',
                 '-a' => 'asciidoc-dir=' . $asciidoc_dir,
+                $resources ? ( '-a' => 'resources=' . join(',', @$resources)) : (),
                 '--destination-dir=' . $dest,
                 docinfo($index),
                 $index
@@ -108,7 +107,6 @@ sub build_chunked {
                 file('resources/website_chunked.xsl')->absolute,
                 "$dest/index.xml"
             );
-            # TODO copy_resources?
             unlink "$dest/index.xml";
             1;
         } or do { $output = $@; $died = 1; };
@@ -205,15 +203,14 @@ sub build_single {
             $output = run(
                 'asciidoctor', '-v', '--trace',
                 '-r' => dir('resources/asciidoctor/lib/extensions.rb')->absolute,
-                # TODO figure out resource
-                # ( map { ( '--resource' => $_ ) } @$resources ),
                 '-b' => 'docbook45',
                 '-d' => $type,
                 '-a' => 'showcomments=1',
                 '-a' => "lang=$lang",
                 '-a' => 'repo_root=' . $root_dir,
                 $private ? () : ( '-a' => "edit_url=$edit_url" ),
                 '-a' => 'asciidoc-dir=' . $asciidoc_dir,
+                $resources ? ( '-a' => 'resources=' . join(',', @$resources)) : (),
                 # Disable warning on missing attributes because we have
                 # missing attributes!
                 # '-a' => 'attribute-missing=warn',
@@ -231,7 +228,6 @@ sub build_single {
                 file('resources/website.xsl')->absolute,
                 "$dest/index.xml"
             );
-            # TODO copy_resources?
             unlink "$dest/index.xml";
             1;
         } or do { $output = $@; $died = 1; };

resources/asciidoctor/lib/copy_images/extension.rb

@@ -0,0 +1,69 @@
+require 'csv'
+require 'fileutils'
+require 'set'
+require_relative '../scaffold.rb'
+
+include Asciidoctor
+
+##
+# Copies images that are referenced into the same directory as the output files.
+#
+class CopyImages < TreeProcessorScaffold
+  include Logging
+
+  def initialize name
+    super
+    @copied = Set[]
+  end
+
+  def process_block block
+    return unless block.context == :image
+    uri = block.image_uri(block.attr 'target')
+    return if Helpers.uriish? uri       # Skip external images
+    return unless @copied.add? uri      # Skip images we've copied before
+    source = find_source block, uri
+    return unless source                # Skip images we can't find
+    logger.info message_with_context "copying #{uri}", :source_location => block.source_location
+    copy_image_proc = block.document.attr 'copy_image'
+    if copy_image_proc
+      # Delegate to a proc for copying if one is defined. Used for testing.
+      copy_image_proc.call(uri, source)
+    else
+      destination = ::File.join block.document.options[:to_dir], uri
+      destination_dir = ::File.dirname destination
+      FileUtils.mkdir_p destination_dir
+      FileUtils.cp source, destination
+    end
+  end
+
+  ##
+  # Does a breadth first search starting at the base_dir of the document and
+  # any referenced resources. This isn't super efficient but it is how a2x works
+  # and we strive for compatibility.
+  #
+  def find_source block, uri
+    to_check = [block.document.base_dir]
+    checked = []
+
+    resources = block.document.attr 'resources'
+    if resources
+      to_check += CSV.parse_line(resources)
+    end
+
+    while (dir = to_check.shift)
+      checked << block.normalize_system_path(uri, dir)
+      return checked.last if File.readable? checked.last
+      next unless Dir.exist?(dir)
+      Dir.new(dir).each { |f|
+        next if f == '.' || f == '..'
+        f = File.join(dir, f)
+        to_check << f if File.directory?(f)
+      }
+    end
+
+    # We'll skip images we can't find but we should log something about it so
+    # we can fix them.
+    logger.warn message_with_context "can't read image at any of #{checked}", :source_location => block.source_location
+    nil
+  end
+end

resources/asciidoctor/lib/extensions.rb

@@ -1,4 +1,5 @@
 require_relative 'added/extension'
+require_relative 'copy_images/extension'
 require_relative 'cramped_include/extension'
 require_relative 'edit_me/extension'
 require_relative 'elastic_compat_tree_processor/extension'
@@ -12,6 +13,7 @@
   document.sourcemap = true
   preprocessor CrampedInclude
   preprocessor ElasticCompatPreprocessor
+  treeprocessor CopyImages
   treeprocessor EditMe
   treeprocessor ElasticCompatTreeProcessor
   include_processor ElasticIncludeTagged

resources/asciidoctor/spec/copy_images_spec.rb

@@ -0,0 +1,197 @@
+require 'copy_images/extension'
+require 'fileutils'
+require 'tmpdir'
+
+RSpec.describe CopyImages do
+  RSpec::Matchers.define_negated_matcher :not_match, :match
+
+  before(:each) do
+    Extensions.register do
+      tree_processor CopyImages
+    end
+  end
+
+  after(:each) do
+    Extensions.unregister_all
+  end
+
+  private
+  def copy_attributes copied
+    return {
+      'copy_image' => Proc.new { |uri, source|
+        copied << [uri, source]
+      }
+    }
+  end
+
+  spec_dir = File.dirname(__FILE__)
+
+  it "copies a file when directly referenced" do
+    copied = []
+    attributes = copy_attributes copied
+    input = <<~ASCIIDOC
+      == Example
+      image::resources/copy_images/example1.png[]
+    ASCIIDOC
+    convert input, attributes, match(/INFO: <stdin>: line 2: copying resources\/copy_images\/example1.png/)
+    expect(copied).to eq([
+        ["resources/copy_images/example1.png", "#{spec_dir}/resources/copy_images/example1.png"]
+    ])
+  end
+
+  it "copies a file when it can be found in a sub tree" do
+    copied = []
+    attributes = copy_attributes copied
+    input = <<~ASCIIDOC
+      == Example
+      image::example1.png[]
+    ASCIIDOC
+    convert input, attributes, match(/INFO: <stdin>: line 2: copying example1.png/)
+    expect(copied).to eq([
+        ["example1.png", "#{spec_dir}/resources/copy_images/example1.png"]
+    ])
+  end
+
+  it "copies a path when it can be found in a sub tree" do
+    copied = []
+    attributes = copy_attributes copied
+    input = <<~ASCIIDOC
+      == Example
+      image::copy_images/example1.png[]
+    ASCIIDOC
+    convert input, attributes, match(/INFO: <stdin>: line 2: copying copy_images\/example1.png/)
+    expect(copied).to eq([
+        ["copy_images/example1.png", "#{spec_dir}/resources/copy_images/example1.png"]
+    ])
+  end
+
+  it "warns when it can't find a file" do
+    copied = []
+    attributes = copy_attributes copied
+    input = <<~ASCIIDOC
+      == Example
+      image::not_found.jpg[]
+    ASCIIDOC
+    convert input, attributes, match(/
+        WARN:\ <stdin>:\ line\ 2:\ can't\ read\ image\ at\ any\ of\ \[
+          "#{spec_dir}\/not_found.jpg",\s
+          "#{spec_dir}\/resources\/not_found.jpg",\s
+          .+
+          "#{spec_dir}\/resources\/copy_images\/not_found.jpg"
+          .+
+        \]/x).and(not_match(/INFO: <stdin>/))
+    expect(copied).to eq([])
+  end
+
+  it "only attempts to copy each file once" do
+    copied = []
+    attributes = copy_attributes copied
+    input = <<~ASCIIDOC
+      == Example
+      image::resources/copy_images/example1.png[]
+      image::resources/copy_images/example1.png[]
+      image::resources/copy_images/example2.png[]
+      image::resources/copy_images/example1.png[]
+      image::resources/copy_images/example2.png[]
+      ASCIIDOC
+    convert input, attributes, match(/INFO: <stdin>: line 2: copying resources\/copy_images\/example1.png/).and(
+        match(/INFO: <stdin>: line 4: copying resources\/copy_images\/example2.png/))
+    expect(copied).to eq([
+        ["resources/copy_images/example1.png", "#{spec_dir}/resources/copy_images/example1.png"],
+        ["resources/copy_images/example2.png", "#{spec_dir}/resources/copy_images/example2.png"],
+    ])
+  end
+
+  it "skips external images" do
+    copied = []
+    attributes = copy_attributes copied
+    input = <<~ASCIIDOC
+      == Example
+      image::https://f.cloud.github.com/assets/4320215/768165/19d8b1aa-e899-11e2-91bc-6b0553e8d722.png[]
+      ASCIIDOC
+    convert input, attributes
+    expect(copied).to eq([])
+  end
+
+  it "can find files using a single valued resources attribute" do
+    Dir.mktmpdir {|tmp|
+      FileUtils.cp(
+          ::File.join(spec_dir, 'resources', 'copy_images', 'example1.png'),
+          ::File.join(tmp, 'tmp_example1.png')
+      )
+
+      copied = []
+      attributes = copy_attributes copied
+      attributes['resources'] = tmp
+      input = <<~ASCIIDOC
+        == Example
+        image::tmp_example1.png[]
+      ASCIIDOC
+      # NOCOMMIT full paths in logs too, I think
+      convert input, attributes, match(/INFO: <stdin>: line 2: copying tmp_example1.png/)
+      expect(copied).to eq([
+          ["tmp_example1.png", "#{tmp}/tmp_example1.png"]
+      ])
+    }
+  end
+
+  it "can find files using a multi valued resources attribute" do
+    Dir.mktmpdir {|tmp|
+      FileUtils.cp(
+          ::File.join(spec_dir, 'resources', 'copy_images', 'example1.png'),
+          ::File.join(tmp, 'tmp_example1.png')
+      )
+
+      copied = []
+      attributes = copy_attributes copied
+      attributes['resources'] = "dummy1,#{tmp},/dummy2"
+      input = <<~ASCIIDOC
+        == Example
+        image::tmp_example1.png[]
+      ASCIIDOC
+      convert input, attributes, match(/INFO: <stdin>: line 2: copying tmp_example1.png/)
+      expect(copied).to eq([
+          ["tmp_example1.png", "#{tmp}/tmp_example1.png"]
+      ])
+    }
+  end
+
+  it "has a nice error message when it can't find a file with single valued resources attribute" do
+    Dir.mktmpdir {|tmp|
+      copied = []
+      attributes = copy_attributes copied
+      attributes['resources'] = tmp
+      input = <<~ASCIIDOC
+        == Example
+        image::not_found.png[]
+      ASCIIDOC
+      convert input, attributes, match(/
+          WARN:\ <stdin>:\ line\ 2:\ can't\ read\ image\ at\ any\ of\ \[
+            "#{spec_dir}\/not_found.png",\s
+            "#{tmp}\/not_found.png"
+            .+
+          \]/x).and(not_match(/INFO: <stdin>/))
+      expect(copied).to eq([])
+    }
+  end
+
+  it "has a nice error message when it can't find a file with multi valued resources attribute" do
+    Dir.mktmpdir {|tmp|
+      copied = []
+      attributes = copy_attributes copied
+      attributes['resources'] = "#{tmp},/dummy2"
+      input = <<~ASCIIDOC
+        == Example
+        image::not_found.png[]
+      ASCIIDOC
+      convert input, attributes, match(/
+          WARN:\ <stdin>:\ line\ 2:\ can't\ read\ image\ at\ any\ of\ \[
+            "#{spec_dir}\/not_found.png",\s
+            "#{tmp}\/not_found.png",\s
+            "\/dummy2\/not_found.png"
+            .+
+          \]/x).and(not_match(/INFO: <stdin>/))
+      expect(copied).to eq([])
+    }
+  end
+end

resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb

@@ -215,11 +215,8 @@
       <screen>foo</screen>
       </chapter>
     DOCBOOK
-    expect { convert(input) }.to raise_error { |error|
-      expect(error).to be_a(ConvertError)
-      expect(error.message).to match(/<stdin>: line 4: code block end doesn't match start/)
-      expect(error.result).to eq(expected.strip)
-    }
+    actual = convert input, {}, match(/<stdin>: line 4: code block end doesn't match start/)
+    expect(actual).to eq(expected.strip)
   end
 
   it "doesn't break table-style outputs" do

resources/asciidoctor/spec/elastic_include_tagged_spec.rb

@@ -86,24 +86,36 @@
     input = <<~ASCIIDOC
       include::elastic-include-tagged:resources/elastic_include_tagged/DoesNotExist.java[doesn't-matter]
     ASCIIDOC
-    expect { convert(input) }.to raise_error(
-        ConvertError, /<stdin>: line 2: include file not found/)
+    expected = <<~DOCBOOK
+      <preface>
+      <title></title>
+      <simpara>Unresolved directive in &lt;stdin&gt; - include::resources/elastic_include_tagged/DoesNotExist.java[{1&#8658;"doesn&#8217;t-matter"}]</simpara>
+      </preface>
+    DOCBOOK
+    actual = convert input, {}, match(/<stdin>: line 2: include file not found/)
+    expect(actual).to eq(expected.strip)
   end
 
   it "warns if the start tag is missing" do
     input = <<~ASCIIDOC
       include::elastic-include-tagged:resources/elastic_include_tagged/Example.java[missing-start]
     ASCIIDOC
-    expect { convert(input) }.to raise_error(
-        ConvertError, /<stdin>: line 2: elastic-include-tagged missing start tag \[missing-start\]/)
+    actual = convert input, {}, match(/<stdin>: line 2: elastic-include-tagged missing start tag \[missing-start\]/)
+    expect(actual).to eq('')
   end
 
   it "warns if the end tag is missing" do
     input = <<~ASCIIDOC
       include::elastic-include-tagged:resources/elastic_include_tagged/Example.java[missing-end]
     ASCIIDOC
-    expect { convert(input) }.to raise_error(
-        ConvertError, /resources\/elastic_include_tagged\/Example.java: line \d+: elastic-include-tagged missing end tag \[missing-end\]/)
+    expected = <<~DOCBOOK
+      <preface>
+      <title></title>
+      <simpara>System.err.println("this tag doesn&#8217;t have any end");</simpara>
+      </preface>
+    DOCBOOK
+    actual = convert input, {}, match(/resources\/elastic_include_tagged\/Example.java: line \d+: elastic-include-tagged missing end tag \[missing-end\]/)
+    expect(actual).to eq(expected.strip)
   end
 
   it "isn't invoked by include-tagged::" do