diff --git a/resources/asciidoctor/lib/change_admonition/extension.rb b/resources/asciidoctor/lib/change_admonition/extension.rb index 72dec1ac4f6d5..351585d35cc02 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_converter.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 fbc16b16d7368..a5ff224766e32 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_converter.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 = { @@ -27,130 +32,79 @@ class CopyImages < TreeProcessorScaffold 'deleted' => 'warning', }.freeze CALLOUT_RX = /CO\d+-(\d+)/ - 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 + #### "Conversion" methods - def process_block_image(block) - return unless block.context == :image - - uri = block.image_uri(block.attr('target')) - process_image block, uri + def admonition(node) + 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 - ## - # 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 + 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 && - block.parent.context == :olist - - block.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 image(node) + copy_image node, node.attr('target') + 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 process_image(block, uri) + #### Helper methods + def copy_image(node, uri) return unless uri return if Asciidoctor::Helpers.uriish? uri # Skip external images - @copier.copy_image block, uri + @copier.copy_image node, uri 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 - - coids = block.attr('coids') + def copy_image_for_callout_items(callout_extension, node) + coids = node.attr('coids') return unless coids coids.scan(CALLOUT_RX) do |(index)| - @copier.copy_image( - block, "images/icons/callouts/#{index}.#{callout_extension}" - ) + path = "images/icons/callouts/#{index}.#{callout_extension}" + @copier.copy_image node, path end end - def process_admonition(block) - admonition_extension = block.document.attr 'copy-admonition-images' - return unless admonition_extension + def admonition_image(node) + if (revisionflag = node.attr 'revisionflag') + image = ADMONITION_IMAGE_FOR_REVISION_FLAG[revisionflag] + return image if image - process_standard_admonition admonition_extension, block - process_change_admonition admonition_extension, block - 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 - - @copier.copy_image( - block, "images/icons/#{style.downcase}.#{admonition_extension}" - ) - end - - def process_change_admonition(admonition_extension, block) - revisionflag = block.attr 'revisionflag' - return unless revisionflag - - 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/lib/delegating_converter.rb b/resources/asciidoctor/lib/delegating_converter.rb new file mode 100644 index 0000000000000..51e2a5a2c5f7f --- /dev/null +++ b/resources/asciidoctor/lib/delegating_converter.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/copy_images_spec.rb b/resources/asciidoctor/spec/copy_images_spec.rb index 940f372dea88b..fb7da1625a38b 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 @@ -64,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. @@ -73,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 @@ -105,16 +104,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 @@ -130,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 .+ @@ -148,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 @@ -177,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 @@ -193,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 .+ @@ -208,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 @@ -227,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' @@ -275,15 +291,62 @@ expect(logs).to eq(expected_logs.strip) end end - context 'when the inline image is inside a list' 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 + 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 an unordered 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 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 + [[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 there is an empty definition list' do + let(:input) do + <<~ASCIIDOC + == Example + Foo:: + Bar::: + ASCIIDOC + end + it "doesn't log anything" do + expect(logs).to eq('') + end end end @@ -485,7 +548,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 @@ -515,7 +578,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' @@ -563,8 +625,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'