diff --git a/lib/ES/Util.pm b/lib/ES/Util.pm index 103de1f6bcfb0..906d1a83d93f7 100644 --- a/lib/ES/Util.pm +++ b/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,8 +203,6 @@ 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', @@ -214,6 +210,7 @@ sub build_single { '-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; }; diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb new file mode 100644 index 0000000000000..803a002a8765c --- /dev/null +++ b/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 diff --git a/resources/asciidoctor/lib/extensions.rb b/resources/asciidoctor/lib/extensions.rb index ca85b05a4fe5b..b7eaa25ee5445 100644 --- a/resources/asciidoctor/lib/extensions.rb +++ b/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 diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb new file mode 100644 index 0000000000000..8300141a52a41 --- /dev/null +++ b/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: : 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: : 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: : 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:\ :\ 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: /)) + 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: : line 2: copying resources\/copy_images\/example1.png/).and( + match(/INFO: : 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: : 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: : 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:\ :\ line\ 2:\ can't\ read\ image\ at\ any\ of\ \[ + "#{spec_dir}\/not_found.png",\s + "#{tmp}\/not_found.png" + .+ + \]/x).and(not_match(/INFO: /)) + 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:\ :\ 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: /)) + expect(copied).to eq([]) + } + end +end \ No newline at end of file diff --git a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb index 8581eee006d78..de39df2e0a759 100644 --- a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb +++ b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb @@ -215,11 +215,8 @@ foo DOCBOOK - expect { convert(input) }.to raise_error { |error| - expect(error).to be_a(ConvertError) - expect(error.message).to match(/: line 4: code block end doesn't match start/) - expect(error.result).to eq(expected.strip) - } + actual = convert input, {}, match(/: line 4: code block end doesn't match start/) + expect(actual).to eq(expected.strip) end it "doesn't break table-style outputs" do diff --git a/resources/asciidoctor/spec/elastic_include_tagged_spec.rb b/resources/asciidoctor/spec/elastic_include_tagged_spec.rb index 93710859c3ac8..dbc4451694922 100644 --- a/resources/asciidoctor/spec/elastic_include_tagged_spec.rb +++ b/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, /: line 2: include file not found/) + expected = <<~DOCBOOK + + + Unresolved directive in <stdin> - include::resources/elastic_include_tagged/DoesNotExist.java[{1⇒"doesn’t-matter"}] + + DOCBOOK + actual = convert input, {}, match(/: 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, /: line 2: elastic-include-tagged missing start tag \[missing-start\]/) + actual = convert input, {}, match(/: 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 + + + System.err.println("this tag doesn’t have any end"); + + 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 diff --git a/resources/asciidoctor/spec/resources/copy_images/example1.png b/resources/asciidoctor/spec/resources/copy_images/example1.png new file mode 100644 index 0000000000000..f37764b1f7606 Binary files /dev/null and b/resources/asciidoctor/spec/resources/copy_images/example1.png differ diff --git a/resources/asciidoctor/spec/resources/copy_images/example2.png b/resources/asciidoctor/spec/resources/copy_images/example2.png new file mode 100644 index 0000000000000..08cd6f2bfd1b5 Binary files /dev/null and b/resources/asciidoctor/spec/resources/copy_images/example2.png differ diff --git a/resources/asciidoctor/spec/shared_examples/does_not_break_line_numbers.rb b/resources/asciidoctor/spec/shared_examples/does_not_break_line_numbers.rb index ac9798c196855..d0c2df72e671a 100644 --- a/resources/asciidoctor/spec/shared_examples/does_not_break_line_numbers.rb +++ b/resources/asciidoctor/spec/shared_examples/does_not_break_line_numbers.rb @@ -5,15 +5,13 @@ --- <1> callout ASCIIDOC - expect { convert(input) }.to raise_error( - ConvertError, /: line 3: no callout found for <1>/) + convert input, {}, match(/: line 3: no callout found for <1>/) end it "doesn't break line numbers in included files" do input = <<~ASCIIDOC include::resources/does_not_break_line_numbers/missing_callout.adoc[] ASCIIDOC - expect { convert(input) }.to raise_error( - ConvertError, /missing_callout.adoc: line 3: no callout found for <1>/) + convert input, {}, match(/missing_callout.adoc: line 3: no callout found for <1>/) end end diff --git a/resources/asciidoctor/spec/spec_helper.rb b/resources/asciidoctor/spec/spec_helper.rb index e226c41d774e8..e365e5ef3fe6b 100644 --- a/resources/asciidoctor/spec/spec_helper.rb +++ b/resources/asciidoctor/spec/spec_helper.rb @@ -12,29 +12,12 @@ end end -## -# Error thrown when the conversion results in a warning. -class ConvertError < Exception - attr_reader :warnings - attr_reader :result - - def initialize warnings, result - super('\n' + - warnings - .map { |l| "#{l[:severity]}: #{l[:message].inspect}" } - .join('\n')) - @warnings = warnings - @result = result - end -end - # Put asciidoctor into verbose mode so it'll log reference errors $VERBOSE = true ## -# Convert an asciidoc string into docbook. If the conversion results in any -# errors or warnings then raises a ConvertError. -def convert input, extra_attributes = {} +# Convert an asciidoc string into docbook. +def convert input, extra_attributes = {}, warnings_matcher = eq('') logger = Asciidoctor::MemoryLogger.new attributes = { 'docdir' => File.dirname(__FILE__), @@ -48,8 +31,9 @@ def convert input, extra_attributes = {} :attributes => attributes, :sourcemap => true, } - if logger.messages != [] then - raise ConvertError.new(logger.messages, result) - end + warnings_string = logger.messages + .map { |l| "#{l[:severity]}: #{l[:message].inspect}" } + .join('\n') + expect(warnings_string).to warnings_matcher result end