From 21e47e8a6e00692218859b39be53ca8322b9973b Mon Sep 17 00:00:00 2001
From: Dan Allen <dan.j.allen@gmail.com>
Date: Sun, 11 Oct 2015 22:15:05 -0600
Subject: [PATCH] code review for #332

- change how admonition icon data is stored in theme
- properly evaluate and convert icon data values
- rename :key to :name in icon hash
- rename :color to :stroke_color in icon hash
- simplify and consolidate logic
---
 lib/asciidoctor-pdf/converter.rb    | 32 ++++++++++++-----------------
 lib/asciidoctor-pdf/theme_loader.rb | 19 +++++------------
 2 files changed, 18 insertions(+), 33 deletions(-)

diff --git a/lib/asciidoctor-pdf/converter.rb b/lib/asciidoctor-pdf/converter.rb
index 790696a93..48a29d870 100644
--- a/lib/asciidoctor-pdf/converter.rb
+++ b/lib/asciidoctor-pdf/converter.rb
@@ -33,11 +33,11 @@ class Converter < ::Prawn::Document
 
   AsciidoctorVersion = ::Gem::Version.create ::Asciidoctor::VERSION
   AdmonitionIcons = {
-    caution:   { key: 'fa-fire', color: 'BF3400', size: 24 },
-    important: { key: 'fa-exclamation-circle', color: 'BF0000', size: 24 },
-    note:      { key: 'fa-info-circle', color: '19407C', size: 24 },
-    tip:       { key: 'fa-lightbulb-o', color: '111111', size: 24 },
-    warning:   { key: 'fa-exclamation-triangle', color: 'BF6900', size: 24 }
+    caution:   { name: 'fa-fire', stroke_color: 'BF3400', size: 24 },
+    important: { name: 'fa-exclamation-circle', stroke_color: 'BF0000', size: 24 },
+    note:      { name: 'fa-info-circle', stroke_color: '19407C', size: 24 },
+    tip:       { name: 'fa-lightbulb-o', stroke_color: '111111', size: 24 },
+    warning:   { name: 'fa-exclamation-triangle', stroke_color: 'BF6900', size: 24 }
   }
   Alignments = [:left, :center, :right]
   AlignmentNames = ['left', 'center', 'right']
@@ -435,14 +435,13 @@ def convert_admonition node
                 # FIXME HACK make title in this location look right
                 label_margin_top = node.title? ? @theme.caption_margin_inside : 0
                 if icons
-                  admon_icon_data = admonition_icon_data label
-                  opts = {
+                  icon_data = admonition_icon_data label
+                  icon icon_data[:name], {
                     valign: :center,
                     align: :center,
-                    color: admon_icon_data[:color],
-                    size: (admonition_icon_size node, admon_icon_data[:size])
+                    color: icon_data[:stroke_color],
+                    size: (fit_icon_size node, icon_data[:size])
                   }
-                  icon admon_icon_data[:key], opts
                 else
                   layout_prose label, valign: :center, style: :bold, line_height: 1, margin_top: label_margin_top, margin_bottom: 0
                 end
@@ -1776,18 +1775,13 @@ def layout_toc_level sections, num_levels, line_metrics, dot_width, num_front_ma
 
   # Reduce icon size to fit inside bounds.height. Icons will not render
   # properly if they are larger than the current bounds.height.
-  def admonition_icon_size node, max_size = 24
-    min_height = bounds.height.floor
-    min_height < max_size ? min_height : max_size
+  def fit_icon_size node, max_size = 24
+    (min_height = bounds.height.floor) < max_size ? min_height : max_size
   end
 
   def admonition_icon_data key
-    if @theme.admonition_icons && @theme.admonition_icons[key]
-      @theme.admonition_icons[key].tap do |data|
-        AdmonitionIcons[key].each do |k, v|
-          data[k] ||= v
-        end
-      end
+    if (icon_data = @theme[%(admonition_icon_#{key})])
+      AdmonitionIcons[key].merge icon_data
     else
       AdmonitionIcons[key]
     end
diff --git a/lib/asciidoctor-pdf/theme_loader.rb b/lib/asciidoctor-pdf/theme_loader.rb
index 78bc4d316..091d94dab 100644
--- a/lib/asciidoctor-pdf/theme_loader.rb
+++ b/lib/asciidoctor-pdf/theme_loader.rb
@@ -86,8 +86,11 @@ def load hash, theme_data = nil
   private
 
   def process_entry key, val, data
-    if key == "admonition_icons"
-      data[key] = symbolize_keys val
+    if key.start_with? 'admonition_icon_'
+      data[key] = (val || {}).map do |(key2, val2)|
+        static_val2 = evaluate val2, data
+        [key2.to_sym, (key2.end_with? '_color') ? to_color(static_val2) : static_val2]
+      end.to_h
     elsif key != 'font_catalog' && ::Hash === val
       val.each do |key2, val2|
         process_entry %(#{key}_#{key2.tr '-', '_'}), val2, data
@@ -225,18 +228,6 @@ def to_color value
     end
     HexColorValue.new value.upcase
   end
-
-  def symbolize_keys hash
-    hash.inject({}) do |data, (k, v)|
-      data.tap do |i|
-        if v.is_a? Hash
-          i[k.to_sym] = symbolize_keys v
-        else
-          i[k.to_sym] = v
-        end
-      end
-    end
-  end
 end
 end
 end