From b12f50b9231b4e494a0188911ca2a971727eef24 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 22 Aug 2019 16:54:51 -0400 Subject: [PATCH 01/11] Copy images from unordeed lists We had support for copying images within ordered lists but not within *un*ordered lists. This fixes that. --- resources/asciidoctor/lib/copy_images/extension.rb | 2 +- resources/asciidoctor/spec/copy_images_spec.rb | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index fbc16b16d7368..d8e8459fee11a 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -80,7 +80,7 @@ def process_inline_image_from_source(block) # with this. def process_inline_image_from_converted(block) return unless block.context == :list_item && - block.parent.context == :olist + %i[olist ulist].include?(block.parent.context) block.text.scan(DOCBOOK_IMAGE_RX) do |(target)| # We have to resolve attributes inside the target. But there is a diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 940f372dea88b..ea2e814f127d5 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -275,7 +275,7 @@ expect(logs).to eq(expected_logs.strip) end end - context 'when the inline image is inside a list' do + context 'when the inline image is inside an ordered list' do let(:input) do <<~ASCIIDOC == Example @@ -285,6 +285,16 @@ let(:resolved) { 'example1.png' } include_examples 'copies example1' end + context 'when the inline image is inside an unordered list' do + let(:input) do + <<~ASCIIDOC + == Example + * words image:example1.png[] words + ASCIIDOC + end + let(:resolved) { 'example1.png' } + include_examples 'copies example1' + end end context 'when the same image is referenced more than once' do From 35fcccc2905efdfd0d78426336eb68061c1bb168 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 22 Aug 2019 17:01:01 -0400 Subject: [PATCH 02/11] Definition lists too! --- resources/asciidoctor/Makefile | 2 +- resources/asciidoctor/lib/copy_images/extension.rb | 2 +- resources/asciidoctor/spec/copy_images_spec.rb | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/resources/asciidoctor/Makefile b/resources/asciidoctor/Makefile index 33b2f2902c8c3..6d75bc4bc78ce 100644 --- a/resources/asciidoctor/Makefile +++ b/resources/asciidoctor/Makefile @@ -9,7 +9,7 @@ check: rspec rubocop .PHONY: rspec rspec: - rspec + rspec spec/copy_images_spec.rb .PHONY: rubocop rubocop: diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index d8e8459fee11a..0744b66fb0797 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -80,7 +80,7 @@ def process_inline_image_from_source(block) # with this. def process_inline_image_from_converted(block) return unless block.context == :list_item && - %i[olist ulist].include?(block.parent.context) + %i[dlist olist ulist].include?(block.parent.context) block.text.scan(DOCBOOK_IMAGE_RX) do |(target)| # We have to resolve attributes inside the target. But there is a diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index ea2e814f127d5..b96c324830039 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -295,6 +295,16 @@ let(:resolved) { 'example1.png' } include_examples 'copies example1' end + context 'when the inline image is inside a definition list' do + let(:input) do + <<~ASCIIDOC + == Example + Foo:: words image:example1.png[] words + ASCIIDOC + end + let(:resolved) { 'example1.png' } + include_examples 'copies example1' + end end context 'when the same image is referenced more than once' do From 51d9a9d851c87a419f2020dfb798027e6cec9eae Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 22 Aug 2019 17:16:02 -0400 Subject: [PATCH 03/11] More careful --- resources/asciidoctor/lib/copy_images/extension.rb | 1 + resources/asciidoctor/spec/copy_images_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index 0744b66fb0797..92ad4ec799cf4 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -81,6 +81,7 @@ def process_inline_image_from_source(block) def process_inline_image_from_converted(block) return unless block.context == :list_item && %i[dlist olist ulist].include?(block.parent.context) + return unless block.text block.text.scan(DOCBOOK_IMAGE_RX) do |(target)| # We have to resolve attributes inside the target. But there is a diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index b96c324830039..9b82f56868140 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -307,6 +307,19 @@ end end + context 'when the inline image is inside an empty definition list' do + let(:input) do + <<~ASCIIDOC + == Example + Foo:: + Bar::: + ASCIIDOC + end + it "doesn't copy an images but at least it doesn't crash" do + expect(copied).to eq([]) + end + end + context 'when the same image is referenced more than once' do let(:input) do <<~ASCIIDOC From 09a14c0e7fe0bc052ded10103a2c6a82a607d966 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 23 Aug 2019 10:24:26 -0400 Subject: [PATCH 04/11] Failing tests --- .../asciidoctor/lib/copy_images/extension.rb | 15 +++- .../asciidoctor/spec/copy_images_spec.rb | 87 +++++++++++-------- 2 files changed, 62 insertions(+), 40 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index 92ad4ec799cf4..483a863e75775 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -81,9 +81,18 @@ def process_inline_image_from_source(block) def process_inline_image_from_converted(block) return unless block.context == :list_item && %i[dlist olist ulist].include?(block.parent.context) - return unless block.text - - block.text.scan(DOCBOOK_IMAGE_RX) do |(target)| + return unless block.text? + + # NOCOMMIT: colist too! + # 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.parent.attributes + text = block.text + block.document.clear_playback_attributes block.parent.attributes + + text.scan(DOCBOOK_IMAGE_RX) do |(target)| # 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 diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 9b82f56868140..2411c0fd349c5 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -105,16 +105,32 @@ context 'when the image contains attributes' do let(:target) { 'example1.{ext}' } let(:resolved) { 'example1.png' } - let(:input) do - <<~ASCIIDOC - == Example - :ext: png + context 'when the attribute is close to the image' do + let(:input) do + <<~ASCIIDOC + == Example + :ext: png - #{image_command} - ASCIIDOC + #{image_command} + ASCIIDOC + end + let(:include_line) { 4 } + include_examples 'copies example1' + end + context 'when the attribute is far from the image' do + let(:input) do + <<~ASCIIDOC + == Example + :ext: png + + Words. + + #{image_command} + ASCIIDOC + end + let(:include_line) { 6 } + include_examples 'copies example1' end - let(:include_line) { 4 } - include_examples 'copies example1' end context 'when referencing an external image' do let(:target) do @@ -276,47 +292,44 @@ end end context 'when the inline image is inside an ordered list' do - let(:input) do - <<~ASCIIDOC - == Example - . words image:example1.png[] words - ASCIIDOC - end - let(:resolved) { 'example1.png' } - include_examples 'copies example1' + let(:image_command) { ". Words image:#{target}[] words" } + include_examples 'copies images with various paths' + end + context 'when the inline image is inside an ordered list' do + let(:image_command) { "* Words image:#{target}[] words" } + include_examples 'copies images with various paths' + end + context 'when the inline image is inside a definition list' do + let(:image_command) { "Foo:: Words image:#{target}[] words" } + include_examples 'copies images with various paths' end - context 'when the inline image is inside an unordered list' do + context 'when there is a reference in an ordered list' do let(:input) do <<~ASCIIDOC + [[foo-thing]] == Example - * words image:example1.png[] words + :id: foo + + More words. + + . <<{id}-thing>> ASCIIDOC end - let(:resolved) { 'example1.png' } - include_examples 'copies example1' + it "doesn't log anything" do + expect(logs).to eq('') + end end - context 'when the inline image is inside a definition list' do + context 'when there is an empty definition list' do let(:input) do <<~ASCIIDOC == Example - Foo:: words image:example1.png[] words + Foo:: + Bar::: ASCIIDOC end - let(:resolved) { 'example1.png' } - include_examples 'copies example1' - end - end - - context 'when the inline image is inside an empty definition list' do - let(:input) do - <<~ASCIIDOC - == Example - Foo:: - Bar::: - ASCIIDOC - end - it "doesn't copy an images but at least it doesn't crash" do - expect(copied).to eq([]) + it "doesn't log anything" do + expect(logs).to eq('') + end end end From 62801e02c045dd47383ce2ba9bb02344c554a23d Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 26 Aug 2019 14:31:50 -0400 Subject: [PATCH 05/11] better! --- .../lib/change_admonition/extension.rb | 44 +++-- .../asciidoctor/lib/copy_images/extension.rb | 172 +++++++----------- .../asciidoctor/lib/delegating_conveter.rb | 30 +++ resources/asciidoctor/lib/extensions.rb | 2 +- .../spec/change_admonition_spec.rb | 2 +- .../asciidoctor/spec/copy_images_spec.rb | 4 +- 6 files changed, 125 insertions(+), 129 deletions(-) create mode 100644 resources/asciidoctor/lib/delegating_conveter.rb diff --git a/resources/asciidoctor/lib/change_admonition/extension.rb b/resources/asciidoctor/lib/change_admonition/extension.rb index 72dec1ac4f6d5..28e8076537a3a 100644 --- a/resources/asciidoctor/lib/change_admonition/extension.rb +++ b/resources/asciidoctor/lib/change_admonition/extension.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'asciidoctor/extensions' +require_relative '../delegating_conveter.rb' ## # Extensions for marking when something was added, when it *will* be added, or @@ -25,6 +26,21 @@ def activate(registry) registry.block_macro ChangeAdmonitionBlock.new(revisionflag, tag), name registry.inline_macro ChangeAdmonitionInline.new(revisionflag), name end + DelegatingConverter.setup(registry.document) { |doc| Converter.new doc } + end + + ## + # Properly renders change admonitions. + class Converter < DelegatingConverter + def admonition(node) + return yield unless (flag = node.attr 'revisionflag') + + <<~DOCBOOK.strip + <#{tag_name = node.attr 'name'} revisionflag="#{flag}" revision="#{node.attr 'version'}"> + #{node.content} + + DOCBOOK + end end ## @@ -41,27 +57,15 @@ def initialize(revisionflag, tag) def process(parent, _target, attrs) version = attrs[:version] - # We can *almost* go through the standard :admonition conversion but - # that won't render the revisionflag or the revision. So we have to - # go with this funny compound pass thing. - admon = Asciidoctor::Block.new(parent, :pass, content_model: :compound) - admon << Asciidoctor::Block.new( - admon, :pass, - attributes: { 'revisionflag' => @revisionflag }, - source: tag_source(version) - ) - admon << Asciidoctor::Block.new( - admon, :paragraph, - source: attrs[:passtext], - subs: Asciidoctor::Substitutors::NORMAL_SUBS + Asciidoctor::Block.new( + parent, :admonition, + attributes: { + 'name' => @tag, + 'revisionflag' => @revisionflag, + 'version' => version, + }, + source: attrs[:passtext] ) - admon << Asciidoctor::Block.new(admon, :pass, source: "") - end - - def tag_source(version) - <<~HTML - <#{@tag} revisionflag="#{@revisionflag}" revision="#{version}"> - HTML end end diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index 483a863e75775..4afba0c6e8c5c 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -1,24 +1,29 @@ # frozen_string_literal: true -require_relative '../scaffold.rb' +require_relative '../delegating_conveter.rb' require_relative 'copier.rb' +## +# Copies images that are referenced into the same directory as the +# output files. +# +# It finds the images by looking in a comma separated list of directories +# defined by the `resources` attribute. +# +# It can also be configured to copy the images that number callout lists by +# setting `copy-callout-images` to the file extension of the images to copy. +# +# It can also be configured to copy the that decoration admonitions by +# setting `copy-admonition-images` to the file extension of the images +# to copy. module CopyImages + def self.activate(registry) + DelegatingConverter.setup(registry.document) { |doc| Converter.new doc } + end + ## - # Copies images that are referenced into the same directory as the - # output files. - # - # It finds the images by looking in a comma separated list of directories - # defined by the `resources` attribute. - # - # It can also be configured to copy the images that number callout lists by - # setting `copy-callout-images` to the file extension of the images to copy. - # - # It can also be configured to copy the that decoration admonitions by - # setting `copy-admonition-images` to the file extension of the images - # to copy. - # - class CopyImages < TreeProcessorScaffold + # A Converter implementation that copies images as it sees them. + class Converter < DelegatingConverter include Asciidoctor::Logging ADMONITION_IMAGE_FOR_REVISION_FLAG = { @@ -30,118 +35,77 @@ class CopyImages < TreeProcessorScaffold INLINE_IMAGE_RX = /(\\)?image:([^:\s\[](?:[^\n\[]*[^\s\[])?)\[/m DOCBOOK_IMAGE_RX = %r{}m - def initialize(name) - super + def initialize(delegate) + super(delegate) @copier = Copier.new end - def process_block(block) - process_inline_image_from_source block - process_inline_image_from_converted block - process_block_image block - process_callout block - process_admonition block - end - - def process_block_image(block) - return unless block.context == :image + #### "Conversion" methods - uri = block.image_uri(block.attr('target')) - process_image block, uri + def admonition(node) + if (extension = node.attr 'copy-admonition-images') && + (style = node.attr 'style') + # The image for a standard admonition comes from the style + path = "images/icons/#{style.downcase}.#{extension}" + @copier.copy_image node, path + end + yield end - ## - # Scan the inline image from the asciidoc source. 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. - def process_inline_image_from_source(block) - return unless block.content_model == :simple - - 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 + def colist(node) + if (extension = node.attr 'copy-callout-images') + node.items.each do |item| + copy_image_for_callout_items extension, item + end end + scan_images_from_docbook node, yield end - ## - # Scan the inline image from the generated docbook. It is not nice that - # this is required but there isn't much we can do about it. We *could* - # rewrite all of the image copying to be against the generated docbook - # using this code but I feel like that'd be slower. For now, we'll stick - # with this. - def process_inline_image_from_converted(block) - return unless block.context == :list_item && - %i[dlist olist ulist].include?(block.parent.context) - return unless block.text? - - # NOCOMMIT: colist too! - # 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.parent.attributes - text = block.text - block.document.clear_playback_attributes block.parent.attributes - - text.scan(DOCBOOK_IMAGE_RX) do |(target)| - # 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. - uri = block.image_uri target - process_image block, uri - end + def dlist(node) + scan_images_from_docbook node, yield end - def process_image(block, uri) - return unless uri - return if Asciidoctor::Helpers.uriish? uri # Skip external images + def image(node) + copy_image node, node.attr('target') + yield + end - @copier.copy_image block, uri + def olist(node) + scan_images_from_docbook node, yield end - def process_callout(block) - callout_extension = block.document.attr 'copy-callout-images' - return unless callout_extension - return unless block.parent && block.parent.context == :colist + def paragraph(node) + scan_images_from_docbook node, yield + end - coids = block.attr('coids') - return unless coids + def ulist(node) + scan_images_from_docbook node, yield + end - coids.scan(CALLOUT_RX) do |(index)| - @copier.copy_image( - block, "images/icons/callouts/#{index}.#{callout_extension}" - ) + #### Helper methods + + def scan_images_from_docbook(node, text) + text.scan(DOCBOOK_IMAGE_RX) do |(uri)| + copy_image node, uri end + text end - def process_admonition(block) - admonition_extension = block.document.attr 'copy-admonition-images' - return unless admonition_extension + def copy_image(node, uri) + return unless uri + return if Asciidoctor::Helpers.uriish? uri # Skip external images - process_standard_admonition admonition_extension, block - process_change_admonition admonition_extension, block + @copier.copy_image node, uri end - def process_standard_admonition(admonition_extension, block) - return unless block.context == :admonition - - # The image for a standard admonition comes from the style - style = block.attr 'style' - return unless style + def copy_image_for_callout_items(callout_extension, node) + coids = node.attr('coids') + return unless coids - @copier.copy_image( - block, "images/icons/#{style.downcase}.#{admonition_extension}" - ) + coids.scan(CALLOUT_RX) do |(index)| + path = "images/icons/callouts/#{index}.#{callout_extension}" + @copier.copy_image node, path + end end def process_change_admonition(admonition_extension, block) diff --git a/resources/asciidoctor/lib/delegating_conveter.rb b/resources/asciidoctor/lib/delegating_conveter.rb new file mode 100644 index 0000000000000..51e2a5a2c5f7f --- /dev/null +++ b/resources/asciidoctor/lib/delegating_conveter.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +## +# Abstract base for implementing a converter that implements some conversions +# and delegates the rest to the "next" converter. +class DelegatingConverter + ## + # Setup a converter on a document. + def self.setup(document) + converter = yield document.converter + document.instance_variable_set :@converter, converter + end + + def initialize(delegate) + @delegate = delegate + end + + def convert(node, transform = nil, opts = {}) + # The behavior of this method mirrors Asciidoctor::Base.convert + t = transform || node.node_name + if respond_to? t + send t, node do + # Passes a block that subclasses can call to run the converter chain. + @delegate.convert node, transform, opts + end + else + @delegate.convert node, transform, opts + end + end +end diff --git a/resources/asciidoctor/lib/extensions.rb b/resources/asciidoctor/lib/extensions.rb index 09a6c5fa79c84..518be8999e7d1 100644 --- a/resources/asciidoctor/lib/extensions.rb +++ b/resources/asciidoctor/lib/extensions.rb @@ -14,6 +14,7 @@ Asciidoctor::Extensions.register CareAdmonition Asciidoctor::Extensions.register ChangeAdmonition +Asciidoctor::Extensions.register CopyImages Asciidoctor::Extensions.register do # Enable storing the source locations so we can look at them. This is required # for EditMe to get a nice location. @@ -21,7 +22,6 @@ block_macro LangOverride preprocessor CrampedInclude preprocessor ElasticCompatPreprocessor - treeprocessor CopyImages::CopyImages treeprocessor EditMe treeprocessor ElasticCompatTreeProcessor # The tree processors after this must come after ElasticComptTreeProcessor diff --git a/resources/asciidoctor/spec/change_admonition_spec.rb b/resources/asciidoctor/spec/change_admonition_spec.rb index d265def2eee97..406acc228e7c3 100644 --- a/resources/asciidoctor/spec/change_admonition_spec.rb +++ b/resources/asciidoctor/spec/change_admonition_spec.rb @@ -31,7 +31,7 @@ let(:expected) do <<~DOCBOOK <#{tag} revisionflag="#{revisionflag}" revision="some_version"> - Like 27 + Like 27 DOCBOOK end diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 2411c0fd349c5..84b31b8164023 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -12,9 +12,7 @@ before(:each) do Asciidoctor::Extensions.register CareAdmonition Asciidoctor::Extensions.register ChangeAdmonition - Asciidoctor::Extensions.register do - tree_processor CopyImages::CopyImages - end + Asciidoctor::Extensions.register CopyImages end after(:each) do From 91f78cc8c317de7fc3e7ed6263b6c68ab88d3fe6 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 26 Aug 2019 15:04:02 -0400 Subject: [PATCH 06/11] The other way --- resources/asciidoctor/Makefile | 2 +- .../lib/change_admonition/extension.rb | 2 +- .../asciidoctor/lib/copy_images/extension.rb | 29 ++++++++-------- .../spec/change_admonition_spec.rb | 2 +- .../asciidoctor/spec/copy_images_spec.rb | 34 +++++++++++++------ 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/resources/asciidoctor/Makefile b/resources/asciidoctor/Makefile index 6d75bc4bc78ce..33b2f2902c8c3 100644 --- a/resources/asciidoctor/Makefile +++ b/resources/asciidoctor/Makefile @@ -9,7 +9,7 @@ check: rspec rubocop .PHONY: rspec rspec: - rspec spec/copy_images_spec.rb + rspec .PHONY: rubocop rubocop: diff --git a/resources/asciidoctor/lib/change_admonition/extension.rb b/resources/asciidoctor/lib/change_admonition/extension.rb index 28e8076537a3a..a1861a6c8968f 100644 --- a/resources/asciidoctor/lib/change_admonition/extension.rb +++ b/resources/asciidoctor/lib/change_admonition/extension.rb @@ -37,7 +37,7 @@ def admonition(node) <<~DOCBOOK.strip <#{tag_name = node.attr 'name'} revisionflag="#{flag}" revision="#{node.attr 'version'}"> - #{node.content} + #{node.content} DOCBOOK end diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index 4afba0c6e8c5c..e194eb95ea13e 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -43,11 +43,11 @@ def initialize(delegate) #### "Conversion" methods def admonition(node) - if (extension = node.attr 'copy-admonition-images') && - (style = node.attr 'style') - # The image for a standard admonition comes from the style - path = "images/icons/#{style.downcase}.#{extension}" - @copier.copy_image node, path + if (extension = node.attr 'copy-admonition-images') + if (image = admonition_image node) + path = "images/icons/#{image}.#{extension}" + @copier.copy_image node, path + end end yield end @@ -108,23 +108,22 @@ def copy_image_for_callout_items(callout_extension, node) end end - def process_change_admonition(admonition_extension, block) - revisionflag = block.attr 'revisionflag' - return unless revisionflag + def admonition_image(node) + if (revisionflag = node.attr 'revisionflag') + image = ADMONITION_IMAGE_FOR_REVISION_FLAG[revisionflag] + return image if image - admonition_image = ADMONITION_IMAGE_FOR_REVISION_FLAG[revisionflag] - if admonition_image - @copier.copy_image( - block, "images/icons/#{admonition_image}.#{admonition_extension}" - ) - else logger.warn( message_with_context( "unknow revisionflag #{revisionflag}", - source_location: block.source_location + source_location: node.source_location ) ) + return end + # The image for a standard admonition comes from the style + style = node.attr 'style' + style&.downcase end end end diff --git a/resources/asciidoctor/spec/change_admonition_spec.rb b/resources/asciidoctor/spec/change_admonition_spec.rb index 406acc228e7c3..d265def2eee97 100644 --- a/resources/asciidoctor/spec/change_admonition_spec.rb +++ b/resources/asciidoctor/spec/change_admonition_spec.rb @@ -31,7 +31,7 @@ let(:expected) do <<~DOCBOOK <#{tag} revisionflag="#{revisionflag}" revision="some_version"> - Like 27 + Like 27 DOCBOOK end diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 84b31b8164023..e8c7196cfeaa2 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -62,6 +62,7 @@ ASCIIDOC end let(:include_line) { 2 } + let(:log_line) { include_line + log_offset } ## # Asserts that some `input` causes just the `example1.png` image to # be copied. @@ -71,7 +72,7 @@ end it 'logs that it copied the image' do expect(logs).to include( - "INFO: : line #{include_line}: copying #{spec_dir}/#{example1}" + "INFO: : line #{log_line}: copying #{spec_dir}/#{example1}" ) end end @@ -144,7 +145,7 @@ context "when it can't find a file" do include_examples "when it can't find a file" let(:expected_logs) do - %r{WARN:\ :\ line\ 2:\ can't\ read\ image\ at\ any\ of\ \[ + %r{WARN:\ :\ line\ \d+:\ can't\ read\ image\ at\ any\ of\ \[ "#{spec_dir}/not_found.jpg",\s "#{spec_dir}/resources/not_found.jpg",\s .+ @@ -162,7 +163,7 @@ let(:resolved) { 'example1.png' } it 'logs an error' do expect(logs).to include( - 'ERROR: : line 2: Error loading [resources]: ' \ + "ERROR: : line #{log_line}: Error loading [resources]: " \ 'Unclosed quoted field on line 1.' ) end @@ -191,7 +192,7 @@ end it 'logs that it copied the image' do expect(logs).to eq( - "INFO: : line 2: copying #{tmp}/#{target}" + "INFO: : line #{log_line}: copying #{tmp}/#{target}" ) end end @@ -207,7 +208,7 @@ context "when it can't find a file" do include_examples "when it can't find a file" let(:expected_logs) do - %r{WARN:\ :\ line\ 2:\ can't\ read\ image\ at\ any\ of\ \[ + %r{WARN:\ :\ line\ \d+:\ can't\ read\ image\ at\ any\ of\ \[ "#{tmp}/not_found.jpg",\s "#{spec_dir}/not_found.jpg",\s .+ @@ -222,7 +223,7 @@ context "when it can't find a file" do include_examples "when it can't find a file" let(:expected_logs) do - %r{WARN:\ :\ line\ 2:\ can't\ read\ image\ at\ any\ of\ \[ + %r{WARN:\ :\ line\ \d+:\ can't\ read\ image\ at\ any\ of\ \[ "/dummy1/not_found.jpg",\s "/dummy2/not_found.jpg",\s "#{tmp}/not_found.jpg",\s @@ -241,6 +242,7 @@ end end + let(:log_offset) { 0 } context 'for the image block macro' do let(:image_command) { "image::#{target}[]" } include_examples 'copies images with various paths' @@ -293,7 +295,7 @@ let(:image_command) { ". Words image:#{target}[] words" } include_examples 'copies images with various paths' end - context 'when the inline image is inside an ordered list' do + context 'when the inline image is inside an unordered list' do let(:image_command) { "* Words image:#{target}[] words" } include_examples 'copies images with various paths' end @@ -301,6 +303,19 @@ let(:image_command) { "Foo:: Words image:#{target}[] words" } include_examples 'copies images with various paths' end + context 'when the inline image is inside a callout list' do + let(:image_command) do + <<~ASCIIDOC + ---- + foo <1> + ---- + + <1> word image:#{target}[] word + ASCIIDOC + end + let(:log_offset) { 4 } + include_examples 'copies images with various paths' + end context 'when there is a reference in an ordered list' do let(:input) do <<~ASCIIDOC @@ -529,7 +544,7 @@ end it 'logs that it copied the image' do expect(logs).to eq(<<~LOGS.strip) - INFO#{location}: copying #{absolute_path}/#{admonition_image}.#{copy_admonition_images} + INFO: : line 1: copying #{absolute_path}/#{admonition_image}.#{copy_admonition_images} LOGS end end @@ -559,7 +574,6 @@ #{admonition.upcase}: Words words words. ASCIIDOC end - let(:location) { ': : line 1' } let(:admonition_image) { admonition } context 'for the note admonition' do include_context 'copy-admonition-images examples' @@ -607,8 +621,6 @@ #{admonition}::[some_version] ASCIIDOC end - # Asciidoctor doesn't make the location available to us for logging here. - let(:location) { '' } let(:admonition_image) { 'note' } context 'for the added admonition' do include_context 'copy-admonition-images examples' From 871919547440a138867eea70fec04c2bf1b7fd80 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 27 Aug 2019 13:41:59 -0400 Subject: [PATCH 07/11] IWP --- lib/ES/Util.pm | 2 +- resources/asciidoctor/spec/copy_images_spec.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/ES/Util.pm b/lib/ES/Util.pm index 1066a20fad722..bdbc4768af29c 100644 --- a/lib/ES/Util.pm +++ b/lib/ES/Util.pm @@ -130,7 +130,7 @@ sub build_chunked { file('resources/website_chunked.xsl')->absolute, $dest_xml ); - unlink $dest_xml; + # unlink $dest_xml; 1; } or do { $output = $@; $died = 1; }; } diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index e8c7196cfeaa2..b3297e38a0bf0 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -291,6 +291,10 @@ expect(logs).to eq(expected_logs.strip) end end + context 'when the inline image is totally alone' do + let(:image_command) { "image:#{target}[width=600]" } + include_examples 'copies images with various paths' + end context 'when the inline image is inside an ordered list' do let(:image_command) { ". Words image:#{target}[] words" } include_examples 'copies images with various paths' From fc238500a1308a4cee7c0707111e4d862ff2f594 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 27 Aug 2019 13:57:16 -0400 Subject: [PATCH 08/11] Inline image! --- resources/asciidoctor/lib/copy_images/extension.rb | 7 +++++++ resources/asciidoctor/spec/copy_images_spec.rb | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index e194eb95ea13e..20980e87f737e 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -70,6 +70,13 @@ def image(node) yield end + def inline_image(node) + # Inline images aren't "real" and don't have a source_location so we have + # to get the location from the parent. + copy_image node.parent, node.target + yield + end + def olist(node) scan_images_from_docbook node, yield end diff --git a/resources/asciidoctor/spec/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index b3297e38a0bf0..fb7da1625a38b 100644 --- a/resources/asciidoctor/spec/copy_images_spec.rb +++ b/resources/asciidoctor/spec/copy_images_spec.rb @@ -291,7 +291,7 @@ expect(logs).to eq(expected_logs.strip) end end - context 'when the inline image is totally alone' do + context 'when the inline image has a specified width' do let(:image_command) { "image:#{target}[width=600]" } include_examples 'copies images with various paths' end From 35a23886552c5ba3f947bc2f5cd7f8be457b42c1 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 27 Aug 2019 13:58:36 -0400 Subject: [PATCH 09/11] Simplify! --- .../asciidoctor/lib/copy_images/extension.rb | 28 +------------------ 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index 20980e87f737e..7f6be01f536e2 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -32,8 +32,6 @@ class Converter < DelegatingConverter 'deleted' => 'warning', }.freeze CALLOUT_RX = /CO\d+-(\d+)/ - INLINE_IMAGE_RX = /(\\)?image:([^:\s\[](?:[^\n\[]*[^\s\[])?)\[/m - DOCBOOK_IMAGE_RX = %r{}m def initialize(delegate) super(delegate) @@ -58,11 +56,7 @@ def colist(node) copy_image_for_callout_items extension, item end end - scan_images_from_docbook node, yield - end - - def dlist(node) - scan_images_from_docbook node, yield + yield end def image(node) @@ -77,27 +71,7 @@ def inline_image(node) yield end - def olist(node) - scan_images_from_docbook node, yield - end - - def paragraph(node) - scan_images_from_docbook node, yield - end - - def ulist(node) - scan_images_from_docbook node, yield - end - #### Helper methods - - def scan_images_from_docbook(node, text) - text.scan(DOCBOOK_IMAGE_RX) do |(uri)| - copy_image node, uri - end - text - end - def copy_image(node, uri) return unless uri return if Asciidoctor::Helpers.uriish? uri # Skip external images From 651c0000b9a16d972c537a4d750cc609b293e209 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 27 Aug 2019 13:59:02 -0400 Subject: [PATCH 10/11] Drop test --- lib/ES/Util.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ES/Util.pm b/lib/ES/Util.pm index bdbc4768af29c..1066a20fad722 100644 --- a/lib/ES/Util.pm +++ b/lib/ES/Util.pm @@ -130,7 +130,7 @@ sub build_chunked { file('resources/website_chunked.xsl')->absolute, $dest_xml ); - # unlink $dest_xml; + unlink $dest_xml; 1; } or do { $output = $@; $died = 1; }; } From 1ef76d62d260e6ab66f35a5397b2ea2a2439a3e6 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 27 Aug 2019 14:00:39 -0400 Subject: [PATCH 11/11] Speeling --- resources/asciidoctor/lib/change_admonition/extension.rb | 2 +- resources/asciidoctor/lib/copy_images/extension.rb | 2 +- .../lib/{delegating_conveter.rb => delegating_converter.rb} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename resources/asciidoctor/lib/{delegating_conveter.rb => delegating_converter.rb} (100%) diff --git a/resources/asciidoctor/lib/change_admonition/extension.rb b/resources/asciidoctor/lib/change_admonition/extension.rb index a1861a6c8968f..351585d35cc02 100644 --- a/resources/asciidoctor/lib/change_admonition/extension.rb +++ b/resources/asciidoctor/lib/change_admonition/extension.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'asciidoctor/extensions' -require_relative '../delegating_conveter.rb' +require_relative '../delegating_converter.rb' ## # Extensions for marking when something was added, when it *will* be added, or diff --git a/resources/asciidoctor/lib/copy_images/extension.rb b/resources/asciidoctor/lib/copy_images/extension.rb index 7f6be01f536e2..a5ff224766e32 100644 --- a/resources/asciidoctor/lib/copy_images/extension.rb +++ b/resources/asciidoctor/lib/copy_images/extension.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative '../delegating_conveter.rb' +require_relative '../delegating_converter.rb' require_relative 'copier.rb' ## diff --git a/resources/asciidoctor/lib/delegating_conveter.rb b/resources/asciidoctor/lib/delegating_converter.rb similarity index 100% rename from resources/asciidoctor/lib/delegating_conveter.rb rename to resources/asciidoctor/lib/delegating_converter.rb