Copy images from unordeed and definition lists (#1093)

Entirely reworks how we copy images by adding a new type of extension
to Asciidoctor: delegating converters. Adding a new converter isn't
documented in their user's manual but their code makes it clear that they
expect folks to plug in customized extensions.

These make copying images *much* simpler because it allows us to
intercept the call for inline images correctly *and* it lets us get at
the attribute values.

This mechanism is also useful for customizing how we generate output
and this PR uses it to clean up one particularly gnarly output mechanism
that was also getting in the way of copying images. It is now much more
readable on both the output generation and image copying side.

Author: nik9000

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'}">
+        <simpara>#{node.content}</simpara>
+        </#{tag_name}>
+      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: "</#{@tag}>")
-    end
-
-    def tag_source(version)
-      <<~HTML
-        <#{@tag} revisionflag="#{@revisionflag}" revision="#{version}">
-      HTML
     end
   end
 

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{<imagedata fileref="([^"]+)"/>}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

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

resources/asciidoctor/lib/extensions.rb

@@ -14,14 +14,14 @@
 
 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.
   document.sourcemap = true
   block_macro LangOverride
   preprocessor CrampedInclude
   preprocessor ElasticCompatPreprocessor
-  treeprocessor CopyImages::CopyImages
   treeprocessor EditMe
   treeprocessor ElasticCompatTreeProcessor
   # The tree processors after this must come after ElasticComptTreeProcessor