From 456229f2c73f2a1424dd0119a0b2f5738b2a1888 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 20 Mar 2019 09:35:40 -0400 Subject: [PATCH 1/4] Asciidoctor: Copy images for inline image macro This copies images declared "inline". Asciidoctor doesn't give a good way of hooking into these images so we have to scan for them when we walk the tree. It isn't the best solution, but at least we don't have to scan for them after the document is rendered! If we did that we wouldn't get line number information when we can't find images.... Closes #730 --- .../asciidoctor/lib/copy_images/extension.rb | 32 ++++- .../asciidoctor/spec/copy_images_spec.rb | 124 ++++++++++++------ resources/asciidoctor/spec/spec_helper.rb | 21 +++ 3 files changed, 137 insertions(+), 40 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index 5c402f6de641a..bca43b6f61afa 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -26,6 +26,7 @@ class CopyImages < TreeProcessorScaffold 'deleted' => 'warning', }.freeze CALLOUT_RX = /CO\d+-(\d+)/ + INLINE_IMAGE_RX = /(\\)?image:([^:\s\[](?:[^\n\[]*[^\s\[])?)\[/m def initialize(name) super @@ -33,15 +34,42 @@ def initialize(name) end def process_block(block) - process_image block + process_inline_image block + process_block_image block process_callout block process_admonition block end - def process_image(block) + def process_block_image(block) return unless block.context == :image uri = block.image_uri(block.attr 'target') + process_image block, uri + end + + def process_inline_image(block) + return unless block.content_model == :simple + + # One day Asciidoc will parse inline things into the AST and we can + # get at them nicely. Today, we have to scrape them from the source + # of the node. + block.source.scan(INLINE_IMAGE_RX) do |(escape, target)| + next if escape + + # We have to resolve attributes inside the target. But there is a + # "funny" ritual for that because attribute substitution is always + # against the document. We have to play the block's attributes against + # the document, then clear them on the way out. + block.document.playback_attributes block.attributes + target = block.sub_attributes target + block.document.clear_playback_attributes block.attributes + uri = block.image_uri target + process_image block, uri + end + end + + def process_image(block, uri) + return if uri == '' return if Asciidoctor::Helpers.uriish? uri # Skip external images @copier.copy_image block, uri diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index cfb7b07da3d62..450e6fea0bd4a 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -6,7 +6,7 @@ require 'fileutils' require 'tmpdir' -RSpec.describe CopyImages::CopyImages do +RSpec.describe CopyImages do RSpec::Matchers.define_negated_matcher :not_match, :match before(:each) do @@ -31,46 +31,94 @@ def copy_attributes(copied) 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, - eq("INFO: : line 2: copying #{spec_dir}\/resources\/copy_images\/example1.png") - expect(copied).to eq([ - ["resources/copy_images/example1.png", "#{spec_dir}/resources/copy_images/example1.png"], - ]) + shared_context 'convert' do + let(:result) do + copied = [] + attributes = copy_attributes copied + result = convert_with_logs input, attributes + result.push copied + result + end + let(:logs) { result[1] } + let(:copied) { result[2] } 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, - eq("INFO: : line 2: copying #{spec_dir}/resources/copy_images/example1.png") - expect(copied).to eq([ - ["example1.png", "#{spec_dir}/resources/copy_images/example1.png"], - ]) + shared_examples 'copies the image' do + include_context 'convert' + it 'copies the image' do + expect(copied).to eq([ + [image_uri, "#{spec_dir}/resources/copy_images/example1.png"], + ]) + end + it 'logs that it copied the image' do + expect(logs).to eq("INFO: : line #{include_line}: copying #{spec_dir}\/resources\/copy_images\/example1.png") + end + end + shared_examples 'copies images' do + let(:input) do + <<~ASCIIDOC + == Example + #{image_command} + ASCIIDOC + end + let(:include_line) { 2 } + # NOCOMMIT rename image_ref and image_uri. target and resolved? + context 'when the image ref matches that path exactly' do + let(:image_ref) { 'resources/copy_images/example1.png' } + let(:image_uri) { 'resources/copy_images/example1.png' } + include_examples 'copies the image' + end + context 'when the image ref is just the name of the image' do + let(:image_ref) { 'example1.png' } + let(:image_uri) { 'example1.png' } + include_examples 'copies the image' + end + context 'when the image ref matches the end of the path' do + let(:image_ref) { 'copy_images/example1.png' } + let(:image_uri) { 'copy_images/example1.png' } + include_examples 'copies the image' + end + context 'when the image contains attributes' do + let(:image_ref) { 'example1.{ext}' } + let(:image_uri) { 'example1.png' } + let(:input) do + <<~ASCIIDOC + == Example + :ext: png + + #{image_command} + ASCIIDOC + end + let(:include_line) { 4 } + include_examples 'copies the image' + end 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, - eq("INFO: : line 2: copying #{spec_dir}/resources/copy_images/example1.png") - expect(copied).to eq([ - ["copy_images/example1.png", "#{spec_dir}/resources/copy_images/example1.png"], - ]) + context 'for the image block macro' do + let(:image_command) { "image::#{image_ref}[]" } + include_examples 'copies images' + end + context 'for the image inline macro' do + # NOCMOMIT rename image_ref to target + # NOCOMMIT replace parameters with let + let(:image_command) { "Words image:#{image_ref}[] words" } + include_examples 'copies images' + context 'when the macro is escaped' do + let(:image_ref) { 'example1.jpg' } + let(:input) do + <<~ASCIIDOC + == Example + "Words \\image:#{image_ref}[] words" + ASCIIDOC + end + include_context 'convert' + it "doesn't log about copying the image" do + expect(logs).to eq('') + end + it "doesn't copy the image" do + expect(copied).to eq([]) + end + end + # NOCOMMIT multiple images in one line end it "warns when it can't find a file" do diff --git a/resources/asciidoctor/spec/spec_helper.rb b/resources/asciidoctor/spec/spec_helper.rb index de79b87727460..f146ea456d80b 100644 --- a/resources/asciidoctor/spec/spec_helper.rb +++ b/resources/asciidoctor/spec/spec_helper.rb @@ -36,3 +36,24 @@ def convert(input, extra_attributes = {}, warnings_matcher = eq('')) expect(warnings_string).to warnings_matcher result end + +## +# Convert an asciidoc string into docbook returning the result and any warnings. +def convert_with_logs(input, extra_attributes = {}) + logger = Asciidoctor::MemoryLogger.new + attributes = { + 'docdir' => File.dirname(__FILE__), + } + attributes.merge! extra_attributes + converted = Asciidoctor.convert input, + :safe => :unsafe, # Used to include "funny" files. + :backend => :docbook45, + :logger => logger, + :doctype => :book, + :attributes => attributes, + :sourcemap => true + logs = logger.messages + .map { |l| "#{l[:severity]}: #{l[:message].inspect}" } + .join("\n") + [converted, logs] +end From feabefbf5605b4a7c34bc134af4cdc91c3c8df57 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 25 Mar 2019 15:00:36 -0400 Subject: [PATCH 2/4] NOCOMMITs --- .../asciidoctor/spec/copy_images_spec.rb | 57 +++++++++++++------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 450e6fea0bd4a..dfcc19981cc91 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -46,11 +46,11 @@ def copy_attributes(copied) include_context 'convert' it 'copies the image' do expect(copied).to eq([ - [image_uri, "#{spec_dir}/resources/copy_images/example1.png"], + [resolved, "#{spec_dir}/resources/copy_images/example1.png"], ]) end it 'logs that it copied the image' do - expect(logs).to eq("INFO: : line #{include_line}: copying #{spec_dir}\/resources\/copy_images\/example1.png") + expect(logs).to eq("INFO: : line #{include_line}: copying #{spec_dir}/resources/copy_images/example1.png") end end shared_examples 'copies images' do @@ -61,25 +61,24 @@ def copy_attributes(copied) ASCIIDOC end let(:include_line) { 2 } - # NOCOMMIT rename image_ref and image_uri. target and resolved? context 'when the image ref matches that path exactly' do - let(:image_ref) { 'resources/copy_images/example1.png' } - let(:image_uri) { 'resources/copy_images/example1.png' } + let(:target) { 'resources/copy_images/example1.png' } + let(:resolved) { 'resources/copy_images/example1.png' } include_examples 'copies the image' end context 'when the image ref is just the name of the image' do - let(:image_ref) { 'example1.png' } - let(:image_uri) { 'example1.png' } + let(:target) { 'example1.png' } + let(:resolved) { 'example1.png' } include_examples 'copies the image' end context 'when the image ref matches the end of the path' do - let(:image_ref) { 'copy_images/example1.png' } - let(:image_uri) { 'copy_images/example1.png' } + let(:target) { 'copy_images/example1.png' } + let(:resolved) { 'copy_images/example1.png' } include_examples 'copies the image' end context 'when the image contains attributes' do - let(:image_ref) { 'example1.{ext}' } - let(:image_uri) { 'example1.png' } + let(:target) { 'example1.{ext}' } + let(:resolved) { 'example1.png' } let(:input) do <<~ASCIIDOC == Example @@ -94,20 +93,18 @@ def copy_attributes(copied) end context 'for the image block macro' do - let(:image_command) { "image::#{image_ref}[]" } + let(:image_command) { "image::#{target}[]" } include_examples 'copies images' end context 'for the image inline macro' do - # NOCMOMIT rename image_ref to target - # NOCOMMIT replace parameters with let - let(:image_command) { "Words image:#{image_ref}[] words" } + let(:image_command) { "Words image:#{target}[] words" } include_examples 'copies images' context 'when the macro is escaped' do - let(:image_ref) { 'example1.jpg' } + let(:target) { 'example1.jpg' } let(:input) do <<~ASCIIDOC == Example - "Words \\image:#{image_ref}[] words" + "Words \\image:#{target}[] words" ASCIIDOC end include_context 'convert' @@ -118,7 +115,31 @@ def copy_attributes(copied) expect(copied).to eq([]) end end - # NOCOMMIT multiple images in one line + context 'when there are multiple images on a line' do + let(:input) do + <<~ASCIIDOC + == Example + + words image:example1.png[] words words image:example2.png[] words + ASCIIDOC + end + let(:expected_warnings) do + <<~WARNINGS + INFO: : line 3: copying #{spec_dir}/resources/copy_images/example1.png + INFO: : line 3: copying #{spec_dir}/resources/copy_images/example2.png + WARNINGS + end + include_context 'convert' + it 'copies the image' do + expect(copied).to eq([ + ['example1.png', "#{spec_dir}/resources/copy_images/example1.png"], + ['example2.png', "#{spec_dir}/resources/copy_images/example2.png"], + ]) + end + it 'logs that it copied the image' do + expect(logs).to eq(expected_warnings.strip) + end + end end it "warns when it can't find a file" do From 940efa27c0c18dd23d08bcf0f14deb9a6d3c994d Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 29 Mar 2019 15:39:12 -0400 Subject: [PATCH 3/4] Switch to a shared context for conversion --- .../asciidoctor/lib/copy_images/extension.rb | 2 +- .../asciidoctor/spec/copy_images_spec.rb | 82 +++++++++++-------- resources/asciidoctor/spec/spec_helper.rb | 43 ++++++---- 3 files changed, 75 insertions(+), 52 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index bca43b6f61afa..e3079d75147c0 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -69,7 +69,7 @@ def process_inline_image(block) end def process_image(block, uri) - return if uri == '' + return unless uri return if Asciidoctor::Helpers.uriish? uri # Skip external images @copier.copy_image block, uri diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index dfcc19981cc91..bf2dbca960409 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -31,29 +31,25 @@ def copy_attributes(copied) spec_dir = File.dirname(__FILE__) - shared_context 'convert' do - let(:result) do - copied = [] - attributes = copy_attributes copied - result = convert_with_logs input, attributes - result.push copied - result - end - let(:logs) { result[1] } - let(:copied) { result[2] } - end - shared_examples 'copies the image' do + ## + # Like the 'convert' shared context, but also captures any images that + # would be copied by the conversion process to the `convert` array. That + # array contains tuples of the form + # [image_path_from_asciidoc_file, image_path_on_disk] and is in the order + # that the images source be copied. + shared_context 'convert intercepting images' do include_context 'convert' - it 'copies the image' do - expect(copied).to eq([ - [resolved, "#{spec_dir}/resources/copy_images/example1.png"], - ]) - end - it 'logs that it copied the image' do - expect(logs).to eq("INFO: : line #{include_line}: copying #{spec_dir}/resources/copy_images/example1.png") - end + + # [] is the initial value but it is mutated by the conversion + let(:copied) { [].dup } + let(:convert_attributes) { copy_attributes(copied) } end - shared_examples 'copies images' do + + ## + # Asserts that a particular `image_command` copies the appropriate image + # when the image is referred to in many ways. The `image_command` should + # read `target` for the location of the image. + shared_examples 'copies images with various paths' do let(:input) do <<~ASCIIDOC == Example @@ -61,20 +57,36 @@ def copy_attributes(copied) ASCIIDOC end let(:include_line) { 2 } + ## + # Asserts that some `input` causes just the `example1.png` image to be copied. + shared_examples 'copies example1' do + include_context 'convert intercepting images' + let(:expected_logs) do + "INFO: : line #{include_line}: copying #{spec_dir}/resources/copy_images/example1.png" + end + it 'copies the image' do + expect(copied).to eq([ + [resolved, "#{spec_dir}/resources/copy_images/example1.png"], + ]) + end + it 'logs that it copied the image' do + expect(logs).to eq(expected_logs) + end + end context 'when the image ref matches that path exactly' do let(:target) { 'resources/copy_images/example1.png' } let(:resolved) { 'resources/copy_images/example1.png' } - include_examples 'copies the image' + include_examples 'copies example1' end context 'when the image ref is just the name of the image' do let(:target) { 'example1.png' } let(:resolved) { 'example1.png' } - include_examples 'copies the image' + include_examples 'copies example1' end context 'when the image ref matches the end of the path' do let(:target) { 'copy_images/example1.png' } let(:resolved) { 'copy_images/example1.png' } - include_examples 'copies the image' + include_examples 'copies example1' end context 'when the image contains attributes' do let(:target) { 'example1.{ext}' } @@ -88,17 +100,17 @@ def copy_attributes(copied) ASCIIDOC end let(:include_line) { 4 } - include_examples 'copies the image' + include_examples 'copies example1' end end context 'for the image block macro' do let(:image_command) { "image::#{target}[]" } - include_examples 'copies images' + include_examples 'copies images with various paths' end context 'for the image inline macro' do let(:image_command) { "Words image:#{target}[] words" } - include_examples 'copies images' + include_examples 'copies images with various paths' context 'when the macro is escaped' do let(:target) { 'example1.jpg' } let(:input) do @@ -107,8 +119,8 @@ def copy_attributes(copied) "Words \\image:#{target}[] words" ASCIIDOC end - include_context 'convert' - it "doesn't log about copying the image" do + include_context 'convert intercepting images' + it "doesn't log anything" do expect(logs).to eq('') end it "doesn't copy the image" do @@ -123,21 +135,21 @@ def copy_attributes(copied) words image:example1.png[] words words image:example2.png[] words ASCIIDOC end - let(:expected_warnings) do - <<~WARNINGS + let(:expected_logs) do + <<~LOGS INFO: : line 3: copying #{spec_dir}/resources/copy_images/example1.png INFO: : line 3: copying #{spec_dir}/resources/copy_images/example2.png - WARNINGS + LOGS end - include_context 'convert' - it 'copies the image' do + include_context 'convert intercepting images' + it 'copies the images' do expect(copied).to eq([ ['example1.png', "#{spec_dir}/resources/copy_images/example1.png"], ['example2.png', "#{spec_dir}/resources/copy_images/example2.png"], ]) end it 'logs that it copied the image' do - expect(logs).to eq(expected_warnings.strip) + expect(logs).to eq(expected_logs.strip) end end end diff --git a/resources/asciidoctor/spec/spec_helper.rb b/resources/asciidoctor/spec/spec_helper.rb index f146ea456d80b..6819c4fd3a1d2 100644 --- a/resources/asciidoctor/spec/spec_helper.rb +++ b/resources/asciidoctor/spec/spec_helper.rb @@ -38,22 +38,33 @@ def convert(input, extra_attributes = {}, warnings_matcher = eq('')) end ## -# Convert an asciidoc string into docbook returning the result and any warnings. -def convert_with_logs(input, extra_attributes = {}) - logger = Asciidoctor::MemoryLogger.new - attributes = { - 'docdir' => File.dirname(__FILE__), - } - attributes.merge! extra_attributes - converted = Asciidoctor.convert input, - :safe => :unsafe, # Used to include "funny" files. - :backend => :docbook45, - :logger => logger, - :doctype => :book, - :attributes => attributes, - :sourcemap => true - logs = logger.messages +# Converts asciidoc to docbook +# +# In: +# input - asciidoc text to convert +# extra_attributes - attributes added to the conversion - defaults to {} +# +# Out: +# converted - converted docbook text +# logs - lines logged +RSpec.shared_context 'convert' do + let(:convert_logger) { Asciidoctor::MemoryLogger.new } + let!(:converted) do + attributes = { + 'docdir' => File.dirname(__FILE__), + } + attributes.merge! convert_attributes if defined?(convert_attributes) + Asciidoctor.convert input, + :safe => :unsafe, # Used to include "funny" files. + :backend => :docbook45, + :logger => convert_logger, + :doctype => :book, + :attributes => attributes, + :sourcemap => true + end + let(:logs) do + convert_logger.messages .map { |l| "#{l[:severity]}: #{l[:message].inspect}" } .join("\n") - [converted, logs] + end end From 9cfaf201c1cec0d5147eb51fc15ff05461c6c3f9 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 29 Mar 2019 15:44:10 -0400 Subject: [PATCH 4/4] Explain ! --- resources/asciidoctor/spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/asciidoctor/spec/spec_helper.rb b/resources/asciidoctor/spec/spec_helper.rb index 6819c4fd3a1d2..0792e4154e249 100644 --- a/resources/asciidoctor/spec/spec_helper.rb +++ b/resources/asciidoctor/spec/spec_helper.rb @@ -50,6 +50,7 @@ def convert(input, extra_attributes = {}, warnings_matcher = eq('')) RSpec.shared_context 'convert' do let(:convert_logger) { Asciidoctor::MemoryLogger.new } let!(:converted) do + # We use let! here to force the conversion because it populates the logger attributes = { 'docdir' => File.dirname(__FILE__), }