refactor formatted text transform to simplify how inner space is collapsed

3 files changed, 66 insertions, 28 deletions

diff --git a/lib/asciidoctor/pdf/formatted_text/transform.rb b/lib/asciidoctor/pdf/formatted_text/transform.rb
index f964f4e0..5ca7c1de 100644
--- a/lib/asciidoctor/pdf/formatted_text/transform.rb
+++ b/lib/asciidoctor/pdf/formatted_text/transform.rb
@@ -6,8 +6,9 @@ module Asciidoctor
       class Transform
         include TextTransformer
 
-        LF = ?\n
+        DummyText = ?\u0000
         ZeroWidthSpace = ?\u200b
+        LF = ?\n + ZeroWidthSpace # without trailing character, use of fallback font can change line height
         CharEntityTable = { amp: '&', apos: ?', gt: '>', lt: '<', nbsp: ?\u00a0, quot: '"' }
         CharRefRx = /&(?:(#{CharEntityTable.keys.join '|'})|#(?:(\d\d\d{0,4})|x(\h\h\h{0,3})));/
         HexColorRx = /^#\h\h\h\h{0,3}$/
@@ -153,6 +154,7 @@ module Asciidoctor
 
         def apply parsed, fragments = [], inherited = nil
           previous_fragment_is_text = false
+          previous_fragment_end_with_space = false
           # NOTE: we use each since using inject is slower than a manual loop
           parsed.each do |node|
             case node[:type]
@@ -160,27 +162,23 @@ module Asciidoctor
               # case 1: non-void element
               if node.key? :pcdata
                 # NOTE: skip element if it has no children
-                if (pcdata = node[:pcdata]).empty?
-                  # QUESTION: should this be handled by the formatter after the transform is complete?
-                  if previous_fragment_is_text && ((previous_fragment_text = fragments[-1][:text]).end_with? ' ')
-                    fragments[-1][:text] = previous_fragment_text.chop
-                  end
-                else
+                unless (pcdata = node[:pcdata]).empty?
                   tag_name = node[:name]
                   attributes = node[:attributes]
-                  parent = clone_fragment inherited
-                  fragment = build_fragment parent, tag_name, attributes
-                  if tag_name == :a && fragment[:type] == :indexterm && !attributes[:visible] &&
-                      previous_fragment_is_text && ((previous_fragment_text = fragments[-1][:text]).end_with? ' ')
-                    fragments[-1][:text] = previous_fragment_text.chop
-                  end
-                  if (text_transform = fragment.delete :text_transform)
-                    text = (text_chunks = extract_text pcdata).join
-                    text_io = StringIO.new transform_text text, text_transform
-                    restore_text pcdata, text_chunks.each_with_object([]) {|chunk, accum| accum << (text_io.read chunk.length) }
+                  fragment = build_fragment (clone_fragment inherited), tag_name, attributes
+                  if tag_name == :a && pcdata[0][:value] == DummyText && pcdata.length == 1
+                    fragment[:text] = DummyText
+                    fragments << fragment
+                  else
+                    if (text_transform = fragment.delete :text_transform)
+                      text = (text_chunks = extract_text pcdata).join
+                      text_io = StringIO.new transform_text text, text_transform
+                      restore_text pcdata, text_chunks.each_with_object([]) {|chunk, accum| accum << (text_io.read chunk.length) }
+                    end
+                    # NOTE: decorate child fragments with inherited properties from this element
+                    apply pcdata, fragments, fragment
+                    previous_fragment_end_with_space = false
                   end
-                  # NOTE: decorate child fragments with inherited properties from this element
-                  apply pcdata, fragments, fragment
                   previous_fragment_is_text = false
                 end
               # case 2: void element
@@ -220,11 +218,11 @@ module Asciidoctor
                     fragment[:image_fit] = img_fit
                   end
                   fragments << fragment
-                  previous_fragment_is_text = false
+                  previous_fragment_is_text = previous_fragment_end_with_space = false
                 else # :br
                   text = @merge_adjacent_text_nodes && previous_fragment_is_text ? %(#{fragments.pop[:text]}#{LF}) : LF
                   fragments << (clone_fragment inherited, text: text)
-                  previous_fragment_is_text = true
+                  previous_fragment_is_text = previous_fragment_end_with_space = true
                 end
               end
             when :charref
@@ -240,10 +238,14 @@ module Asciidoctor
               text = %(#{fragments.pop[:text]}#{text}) if @merge_adjacent_text_nodes && previous_fragment_is_text
               fragments << (clone_fragment inherited, text: text)
               previous_fragment_is_text = true
+              previous_fragment_end_with_space = false
             else # :text
-              text = @merge_adjacent_text_nodes && previous_fragment_is_text ? %(#{fragments.pop[:text]}#{node[:value]}) : node[:value]
-              fragments << (clone_fragment inherited, text: text)
-              previous_fragment_is_text = true
+              unless (text = previous_fragment_end_with_space ? node[:value].lstrip : node[:value]).empty?
+                text = %(#{fragments.pop[:text]}#{text}) if @merge_adjacent_text_nodes && previous_fragment_is_text
+                fragments << (clone_fragment inherited, text: text)
+                previous_fragment_is_text = true
+                previous_fragment_end_with_space = text.end_with? ' '
+              end
             end
           end
           fragments
diff --git a/spec/formatted_text_formatter_spec.rb b/spec/formatted_text_formatter_spec.rb
index 7f5ed68d..2290e8ba 100644
--- a/spec/formatted_text_formatter_spec.rb
+++ b/spec/formatted_text_formatter_spec.rb
@@ -268,6 +268,33 @@ describe Asciidoctor::PDF::FormattedText::Formatter do
       (expect text[0][:string]).to eql 'between'
     end
 
+    it 'should collapse spaces around hidden index term' do
+      pdf = to_pdf 'before (((term))) after', analyze: true
+      text = pdf.text.map {|it| it[:string] }.join
+      (expect text).to eql 'before after'
+    end
+
+    it 'should collapse interspersed newlines around hidden index terms' do
+      pdf = to_pdf %(before\n(((term-a)))\n(((term-b)))\nafter), analyze: true
+      text = pdf.text.map {|it| it[:string] }.join
+      (expect text).to eql 'before after'
+    end
+
+    it 'should use of fallback font after hard line break should not alter line height' do
+      pdf = to_pdf <<~'EOS', attribute_overrides: { 'pdf-theme' => 'default-with-font-fallbacks' }, analyze: true
+      [%hardbreaks]
+      けふこえて
+      あさきゆめみし
+      ゑひもせす
+      EOS
+
+      text = pdf.text
+      (expect text).to have_size 3
+      first_line_gap = (text[1][:y] - text[0][:y]).round 2
+      second_line_gap = (text[2][:y] - text[1][:y]).round 2
+      (expect second_line_gap).to eql first_line_gap
+    end
+
     it 'should format stem equation as monospace' do
       pdf = to_pdf 'Use stem:[x^2] to square the value.', analyze: true
       equation_text = (pdf.find_text 'x^2')[0]
diff --git a/spec/formatted_text_transform_spec.rb b/spec/formatted_text_transform_spec.rb
index 800081dc..57b35a0a 100644
--- a/spec/formatted_text_transform_spec.rb
+++ b/spec/formatted_text_transform_spec.rb
@@ -59,8 +59,17 @@ describe Asciidoctor::PDF::FormattedText::Transform do
     parsed = parser.parse input
     fragments = subject.apply parsed.content
     (expect fragments).to have_size 2
-    (expect fragments[0][:text]).to eql 'foo'
-    (expect fragments[1][:text]).to eql ' bar'
+    (expect fragments[0][:text]).to eql 'foo '
+    (expect fragments[1][:text]).to eql 'bar'
+  end
+
+  it 'should not collapse space only on one side of empty element' do
+    input = 'foo <strong></strong>bar'
+    parsed = parser.parse input
+    fragments = subject.apply parsed.content
+    (expect fragments).to have_size 2
+    (expect fragments[0][:text]).to eql 'foo '
+    (expect fragments[1][:text]).to eql 'bar'
   end
 
   it 'should create fragment with custom font name' do
@@ -150,7 +159,7 @@ describe Asciidoctor::PDF::FormattedText::Transform do
     fragments = subject.apply parsed.content
     (expect fragments).to have_size 3
     (expect fragments[0][:text]).to eql 'foo'
-    (expect fragments[1][:text]).to eql ?\n
+    (expect fragments[1][:text]).to eql %(\n\u200b)
     (expect fragments[2][:text]).to eql 'bar'
   end
 
@@ -159,7 +168,7 @@ describe Asciidoctor::PDF::FormattedText::Transform do
     parsed = parser.parse input
     fragments = (subject.class.new merge_adjacent_text_nodes: true).apply parsed.content
     (expect fragments).to have_size 1
-    (expect fragments[0][:text]).to eql %(foo\nbar)
+    (expect fragments[0][:text]).to eql %(foo\n\u200bbar)
   end
 
   it 'should apply inherited styles' do