Merge pull request #1388 from mojavelinux/issue-1387

resolves #1387 resolve missing attribute in ifeval to empty string

2 files changed, 66 insertions, 31 deletions

diff --git a/lib/asciidoctor/reader.rb b/lib/asciidoctor/reader.rb
index 6565b184..20fb1b25 100644
--- a/lib/asciidoctor/reader.rb
+++ b/lib/asciidoctor/reader.rb
@@ -753,11 +753,14 @@ class PreprocessorReader < Reader
         end
 
         lhs = resolve_expr_val expr_match[1]
-        # regex enforces a restrict set of math-related operations
-        op = expr_match[2]
         rhs = resolve_expr_val expr_match[3]
 
-        skip = !(lhs.send op.to_sym, rhs)
+        # regex enforces a restricted set of math-related operations
+        if (op = expr_match[2]) == '!='
+          skip = lhs.send :==, rhs
+        else
+          skip = !(lhs.send op.to_sym, rhs)
+        end
       end
     end
 
@@ -1126,65 +1129,65 @@ class PreprocessorReader < Reader
   # Examples
   #
   #   expr = '"value"'
-  #   resolve_expr_val(expr)
+  #   resolve_expr_val expr
   #   # => "value"
   #
   #   expr = '"value'
-  #   resolve_expr_val(expr)
+  #   resolve_expr_val expr
   #   # => "\"value"
   #
   #   expr = '"{undefined}"'
-  #   resolve_expr_val(expr)
+  #   resolve_expr_val expr
   #   # => ""
   #
   #   expr = '{undefined}'
-  #   resolve_expr_val(expr)
+  #   resolve_expr_val expr
   #   # => nil
   #
   #   expr = '2'
-  #   resolve_expr_val(expr)
+  #   resolve_expr_val expr
   #   # => 2
   #
   #   @document.attributes['name'] = 'value'
   #   expr = '"{name}"'
-  #   resolve_expr_val(expr)
+  #   resolve_expr_val expr
   #   # => "value"
   #
   # Returns The value of the expression, coerced to the appropriate type
-  def resolve_expr_val(str)
-    val = str
-    type = nil
-
-    if val.start_with?('"') && val.end_with?('"') ||
-        val.start_with?('\'') && val.end_with?('\'')
-      type = :string
+  def resolve_expr_val val
+    if ((val.start_with? '"') && (val.end_with? '"')) ||
+        ((val.start_with? '\'') && (val.end_with? '\''))
+      quoted = true
       val = val[1...-1]
+    else
+      quoted = false
     end
 
     # QUESTION should we substitute first?
+    # QUESTION should we also require string to be single quoted (like block attribute values?)
     if val.include? '{'
-      val = @document.sub_attributes val
+      val = @document.sub_attributes val, :attribute_missing => 'drop'
     end
 
-    unless type == :string
+    if quoted
+      val
+    else
       if val.empty?
-        val = nil
-      elsif val.strip.empty?
-        val = ' '
+        nil
       elsif val == 'true'
-        val = true
+        true
       elsif val == 'false'
-        val = false
-      elsif val.include?('.')
-        val = val.to_f
+        false
+      elsif val.rstrip.empty?
+        ' '
+      elsif val.include? '.'
+        val.to_f
       else
         # fallback to coercing to integer, since we
         # require string values to be explicitly quoted
-        val = val.to_i
+        val.to_i
       end
     end
-
-    val
   end
 
   def include_processors?
diff --git a/test/reader_test.rb b/test/reader_test.rb
index 744ed076..c716c05e 100644
--- a/test/reader_test.rb
+++ b/test/reader_test.rb
@@ -1377,6 +1377,22 @@ content
         end
         assert_equal "ifdef::holygrail[]\ncontent\nendif::holygrail[]", (lines * ::Asciidoctor::EOL)
       end
+
+      test 'ifeval comparing missing attribute to nil is included' do
+        input = <<-EOS
+ifeval::['{foo}' == '']
+No foo for you!
+endif::[]
+        EOS
+
+        doc = Asciidoctor::Document.new input
+        reader = doc.reader
+        lines = []
+        while reader.has_more_lines?
+          lines << reader.read_line
+        end
+        assert_equal 'No foo for you!', (lines * ::Asciidoctor::EOL)
+      end
   
       test 'ifeval comparing double-quoted attribute to matching string is included' do
         input = <<-EOS
@@ -1460,7 +1476,7 @@ endif::[]
   
       test 'ifeval arguments can be transposed' do
         input = <<-EOS
-ifeval::["0.1.0" <= "{asciidoctor-version}"]
+ifeval::['0.1.0' <= '{asciidoctor-version}']
 That version will do!
 endif::[]
         EOS
@@ -1474,14 +1490,30 @@ endif::[]
         assert_equal 'That version will do!', (lines * ::Asciidoctor::EOL)
       end
   
-      test 'ifeval matching numeric comparison is included' do
+      test 'ifeval matching numeric equality is included' do
         input = <<-EOS
 ifeval::[{rings} == 1]
 One ring to rule them all!
 endif::[]
         EOS
   
-        doc = Asciidoctor::Document.new input, :attributes => { 'rings' => 1 }
+        doc = Asciidoctor::Document.new input, :attributes => { 'rings' => '1' }
+        reader = doc.reader
+        lines = []
+        while reader.has_more_lines?
+          lines << reader.read_line
+        end
+        assert_equal 'One ring to rule them all!', (lines * ::Asciidoctor::EOL)
+      end
+
+      test 'ifeval matching numeric inequality is included' do
+        input = <<-EOS
+ifeval::[{rings} != 0]
+One ring to rule them all!
+endif::[]
+        EOS
+  
+        doc = Asciidoctor::Document.new input, :attributes => { 'rings' => '1' }
         reader = doc.reader
         lines = []
         while reader.has_more_lines?