resolves #327 arrange body of article or manpage doctype into multiple columns if page-columns key is set in theme (PR #2232)

9 files changed, 306 insertions, 87 deletions

diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 7a51da10..58da4fb4 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -9,6 +9,10 @@ For a detailed view of what has changed, refer to the {url-repo}/commits/main[co
 
 Enhancements::
 
+* arrange body of article or manpage doctype into multiple columns if `page-columns` key is set in theme (#327)
+* allow column gap to be specified using `page-column-gap` key (#327)
+* introduce `convert_index_categories` method to handle rendering of categories for index inside column box (#327)
+* rename `convert_index_list` method to `convert_index_term` to make its purpose more clear (#327)
 * add `save_theme` helper to work with a copy of the theme within a scope (#2196)
 * add support for `scale` attribute or `iw` unit on `pdfwidth` attribute on image macros (#1933)
 * add backlink from bibref on bibliography entry to first reference to that entry in the document (#1737)
diff --git a/docs/modules/extend/examples/pdf-converter-columns.rb b/docs/modules/extend/examples/pdf-converter-columns.rb
deleted file mode 100644
index b6231bcc..00000000
--- a/docs/modules/extend/examples/pdf-converter-columns.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-class PDFConverterColumns < (Asciidoctor::Converter.for 'pdf')
-  register_for 'pdf'
-
-  def traverse node
-    if node.context == :document &&
-        (columns = ColumnBox === bounds ? 1 : theme.base_columns || 1) > 1
-      column_box [bounds.left, cursor],
-        columns: columns,
-        width: bounds.width,
-        reflow_margins: true,
-        spacer: theme.base_column_gap do
-        super
-      end
-    else
-      super
-    end
-  end
-end
diff --git a/docs/modules/extend/pages/use-cases.adoc b/docs/modules/extend/pages/use-cases.adoc
index 014a29ff..9daa9f2f 100644
--- a/docs/modules/extend/pages/use-cases.adoc
+++ b/docs/modules/extend/pages/use-cases.adoc
@@ -270,34 +270,24 @@ image:
 
 == Multiple columns
 
-Asciidoctor PDF does not yet provide multi-column support, where the body of the article is arranged into multiple columns.
-However, the converter does provide the foundation for supporting a multi-column layout.
-We can tap into that foundation using an extended converter.
+Starting with Asciidoctor PDF 2.1, this converter provides built-in support for multiple columns.
+This feature is available when the doctype is article or manpage, but not book.
+The columns get applied to the body of the document, which excludes the document title and TOC, if present.
 
-The trick is to intercept the `traverse` method and enclose the call in a column box using the `column_box` method.
-The `traverse` method is called to render the body, accepting the document as the sole argument.
-Since this method is also called for other blocks, we'll need to filter out those calls by looking for the `:document` context.
+The Asciidoctor PDF converter also provides the framework for making multi-column layouts in an extended converter.
+This framework is accessible via the helper method `column_box`.
+To make a muti-column layout, you put statements that ink content inside a code block and pass it to the `column_box` method as follows:
 
-.Extended converter that arranges the body into columns
 [,ruby]
 ----
-include::example$pdf-converter-columns.rb[]
+column_box [bounds.left, cursor], columns: 2, width: bounds.width, reflow_margins: true do
+  ink_prose 'left column'
+  bounds.move_past_bottom
+  ink_prose 'right column'
+end
 ----
 
-WARNING: You may encounter some quirks when using this extended converter.
-It's not yet a perfect solution.
-For example, it does not handle the index section correctly.
-You may have to play around with the code to get the desired result.
-
-You can configure the number of columns and the gap between the columns in the theme file as follows:
-
-[,yaml]
-----
-extends: default
-base:
-  columns: 2
-  column-gap: 12
-----
+If you want a multi-column layout for a specific chapter or section, you can override the `traverse` method, look for the section you want to arrange, and wrap the call to `super` in a `column_box` enclosure.
 
 == Theme table using custom role
 
diff --git a/docs/modules/theme/pages/page.adoc b/docs/modules/theme/pages/page.adoc
index 64df40f7..97c5852c 100644
--- a/docs/modules/theme/pages/page.adoc
+++ b/docs/modules/theme/pages/page.adoc
@@ -39,6 +39,21 @@ page:
     recto: image:page-bg-recto.png[]
     verso: image:page-bg-verso.png[]
 
+|columns
+|Integer +
+(default: _not set_)
+|[source]
+page:
+  columns: 2
+
+|column_gap
+|xref:measurement-units.adoc[Measurement] +
+(default: _$base-font-size_)
+|[source]
+page:
+  columns: 2
+  columns_gap: 12
+
 |xref:images.adoc#foreground[foreground-image]
 |xref:images.adoc#specify[image macro] {vbar} xref:images.adoc#specify[path] +
 (default: _not set_)
@@ -111,6 +126,14 @@ If no cover is specified, the recto margin is not applied to the title page.
 To apply the recto margin to the title page, but not include a cover, assign the value `~` to the `front-cover-image` and `back-cover-image` attributes.
 See xref:cover.adoc[] for information about the `cover` category keys.
 
+[#columns]
+=== Columns
+
+The columns are only applied to the body of a document with the article or manpage doctype.
+The body of the document begins after the document title and TOC, if present.
+
+If the columns are set on the index section using the `index-columns` key, they will be ignored when `page-columns` is set.
+
 [#numbering]
 == page-numbering
 
diff --git a/lib/asciidoctor/pdf/converter.rb b/lib/asciidoctor/pdf/converter.rb
index 208a1579..3136dff2 100644
--- a/lib/asciidoctor/pdf/converter.rb
+++ b/lib/asciidoctor/pdf/converter.rb
@@ -171,7 +171,8 @@ module Asciidoctor
         # NOTE: a new page will already be started (page_number = 2) if the front cover image is a PDF
         ink_cover_page doc, :front
         has_front_cover = page_number > marked_page_number
-        if (has_title_page = (title_page_on = doc.doctype == 'book' || (doc.attr? 'title-page')) && (start_title_page doc))
+        doctype = doc.doctype
+        if (has_title_page = (title_page_on = doctype == 'book' || (doc.attr? 'title-page')) && (start_title_page doc))
           # NOTE: the base font must be set before any content is written to the main or scratch document
           font @theme.base_font_family, size: @root_font_size, style: @theme.base_font_style
           if perform_on_single_page { ink_title_page doc }
@@ -280,12 +281,19 @@ module Asciidoctor
           doc.set_attr 'pdf-anchor', (derive_anchor_from_id doc.id, 'top')
           doc.set_attr 'pdf-page-start', page_number
 
-          convert_section generate_manname_section doc if doc.doctype == 'manpage' && (doc.attr? 'manpurpose')
-
-          traverse doc
-
-          # NOTE: for a book, these are leftover footnotes; for an article this is everything
-          outdent_section { ink_footnotes doc }
+          if doctype == 'book' || (columns = @theme.page_columns || 1) < 2
+            convert_section generate_manname_section doc if doctype == 'manpage' && (doc.attr? 'manpurpose')
+            traverse doc
+            # NOTE: for a book, these are leftover footnotes; for an article this is everything
+            outdent_section { ink_footnotes doc }
+          else
+            column_box [bounds.left, cursor], columns: columns, width: bounds.width, reflow_margins: true, spacer: @theme.page_column_gap do
+              convert_section generate_manname_section doc if doctype == 'manpage' && (doc.attr? 'manpurpose')
+              traverse doc
+              # NOTE: for a book, these are leftover footnotes; for an article this is everything
+              outdent_section { ink_footnotes doc }
+            end
+          end
 
           if (toc_extent = @toc_extent)
             if title_page_on && !insert_toc
@@ -710,28 +718,35 @@ module Asciidoctor
       end
 
       def convert_index_section node
-        space_needed_for_category = @theme.description_list_term_spacing + (2 * (height_of_typeset_text 'A'))
-        pagenum_sequence_style = node.document.attr 'index-pagenum-sequence-style'
-        end_cursor = nil
-        column_box [0, cursor], columns: @theme.index_columns, width: bounds.width, reflow_margins: true, spacer: @theme.index_column_gap do
-          @index.categories.each do |category|
-            bounds.move_past_bottom if space_needed_for_category > cursor
-            ink_prose category.name,
-              align: :left,
-              inline_format: false,
-              margin_bottom: @theme.description_list_term_spacing,
-              style: @theme.description_list_term_font_style&.to_sym
-            category.terms.each {|term| convert_index_list_item term, pagenum_sequence_style }
-            @theme.prose_margin_bottom > cursor ? bounds.move_past_bottom : (move_down @theme.prose_margin_bottom)
-          end
-          end_cursor = cursor if bounds.current_column == 0
-        end
-        # Q: could we move this logic into column_box?
-        move_cursor_to end_cursor if end_cursor
+        if ColumnBox === bounds || (columns = @theme.index_columns || 1) < 2
+          convert_index_categories @index.categories, (node.document.attr 'index-pagenum-sequence-style')
+        else
+          end_cursor = nil
+          column_box [bounds.left, cursor], columns: columns, width: bounds.width, reflow_margins: true, spacer: @theme.index_column_gap do
+            convert_index_categories @index.categories, (node.document.attr 'index-pagenum-sequence-style')
+            end_cursor = cursor if bounds.current_column == 0
+          end
+          # Q: could we move this logic into column_box?
+          move_cursor_to end_cursor if end_cursor
+        end
         nil
       end
 
-      def convert_index_list_item term, pagenum_sequence_style = nil
+      def convert_index_categories categories, pagenum_sequence_style = nil
+        space_needed_for_category = @theme.description_list_term_spacing + (2 * (height_of_typeset_text 'A'))
+        categories.each do |category|
+          bounds.move_past_bottom if space_needed_for_category > cursor
+          ink_prose category.name,
+            align: :left,
+            inline_format: false,
+            margin_bottom: @theme.description_list_term_spacing,
+            style: @theme.description_list_term_font_style&.to_sym
+          category.terms.each {|term| convert_index_term term, pagenum_sequence_style }
+          @theme.prose_margin_bottom > cursor ? bounds.move_past_bottom : (move_down @theme.prose_margin_bottom)
+        end
+      end
+
+      def convert_index_term term, pagenum_sequence_style = nil
         term_fragments = term.name.fragments
         unless term.container?
           pagenum_fragment = (parse_text %(<a>#{DummyText}</a>), inline_format: true)[0]
@@ -770,7 +785,7 @@ module Asciidoctor
         typeset_formatted_text term_fragments, (calc_line_metrics @base_line_height), align: :left, color: @font_color, hanging_indent: subterm_indent * 2
         indent subterm_indent do
           term.subterms.each do |subterm|
-            convert_index_list_item subterm, pagenum_sequence_style
+            convert_index_term subterm, pagenum_sequence_style
           end
         end unless term.leaf?
       end
diff --git a/spec/arrange_block_spec.rb b/spec/arrange_block_spec.rb
index 729c1b9c..ac57252e 100644
--- a/spec/arrange_block_spec.rb
+++ b/spec/arrange_block_spec.rb
@@ -1856,16 +1856,9 @@ describe 'Asciidoctor::PDF::Converter#arrange_block' do
     end
 
     it 'should fill extent when block is advanced to next column' do
-      source_file = doc_file 'modules/extend/examples/pdf-converter-columns.rb'
-      source_lines = (File.readlines source_file).select {|l| l == ?\n || (l.start_with? ' ') }
-      ext_class = create_class Asciidoctor::Converter.for 'pdf'
-      backend = %(pdf#{ext_class.object_id})
-      source_lines[0] = %(  register_for '#{backend}'\n)
-      ext_class.class_eval source_lines.join, source_file
-
       pdf_theme.update \
-        base_columns: 2,
-        base_column_gap: 12,
+        page_columns: 2,
+        page_column_gap: 12,
         code_border_radius: 0,
         code_border_width: 0,
         code_background_color: 'EFEFEF'
@@ -1879,7 +1872,7 @@ describe 'Asciidoctor::PDF::Converter#arrange_block' do
         $ asciidoctor-pdf -r asciidoctor-mathematical -a mathematical-format=svg sample.adoc
         ....
         EOS
-        pdf = to_pdf input, backend: backend, pdf_theme: pdf_theme, analyze: true
+        pdf = to_pdf input, pdf_theme: pdf_theme, analyze: true
         pages = pdf.pages
         (expect pages).to have_size 1
         gs = (pdf.extract_graphic_states pages[0][:raw_content])[1]
@@ -1888,14 +1881,7 @@ describe 'Asciidoctor::PDF::Converter#arrange_block' do
     end
 
     it 'should correctly compute to cursor value on extent when column_box starts below top of page' do
-      source_file = doc_file 'modules/extend/examples/pdf-converter-columns.rb'
-      source_lines = (File.readlines source_file).select {|l| l == ?\n || (l.start_with? ' ') }
-      ext_class = create_class Asciidoctor::Converter.for 'pdf'
-      backend = %(pdf#{ext_class.object_id})
-      source_lines[0] = %(  register_for '#{backend}'\n)
-      ext_class.class_eval source_lines.join, source_file
-
-      pdf_theme.update base_columns: 2, base_column_gap: 12, admonition_column_rule_color: '0000FF'
+      pdf_theme.update page_columns: 2, page_column_gap: 12, admonition_column_rule_color: '0000FF'
 
       pdf = with_content_spacer 10, 400 do |spacer_path|
         input = <<~EOS
@@ -1912,10 +1898,10 @@ describe 'Asciidoctor::PDF::Converter#arrange_block' do
         ====
         EOS
 
-        pdf = to_pdf input, backend: backend, pdf_theme: pdf_theme, analyze: true
+        pdf = to_pdf input, pdf_theme: pdf_theme, analyze: true
         pages = pdf.pages
         (expect pages).to have_size 1
-        lines = (to_pdf input, backend: backend, pdf_theme: pdf_theme, analyze: :line).lines
+        lines = (to_pdf input, pdf_theme: pdf_theme, analyze: :line).lines
         column_rules = lines.select {|it| it[:color] == '0000FF' }
         (expect column_rules).to have_size 2
         (expect column_rules[0][:from][:x]).to be < column_rules[1][:from][:x]
diff --git a/spec/index_spec.rb b/spec/index_spec.rb
index 3a8bf3c0..13cdc112 100644
--- a/spec/index_spec.rb
+++ b/spec/index_spec.rb
@@ -818,6 +818,27 @@ describe 'Asciidoctor::PDF::Converter - Index' do
     (expect category_l_text[:x]).to be > category_a_text[:x]
   end
 
+  it 'should ignore index columns if columns are set on page' do
+    pdf = to_pdf <<~EOS, pdf_theme: { page_columns: 2, index_columns: 3 }, analyze: true
+    = Document Title
+    :notitle:
+
+    #{('a'..'z').map {|it| %(((#{it}-keyword))((#{it}-term))) }.join}
+
+    [index]
+    == Index
+    EOS
+
+    midpoint = (get_page_size pdf)[0] * 0.5
+    category_g_text = (pdf.find_text 'A')[0]
+    category_s_text = (pdf.find_text 'L')[0]
+    category_t_text = (pdf.find_text 'W')[0]
+    (expect category_g_text[:page_number]).to be 1
+    (expect category_g_text[:x]).to eql 48.24
+    (expect category_s_text[:x]).to be > midpoint
+    (expect category_t_text[:x]).to eql 48.24
+  end
+
   it 'should not allocate space for anchor if font is missing glyph for null character' do
     pdf_theme = {
       extends: 'default',
diff --git a/spec/manpage_spec.rb b/spec/manpage_spec.rb
index 5d832e23..a3e320e1 100644
--- a/spec/manpage_spec.rb
+++ b/spec/manpage_spec.rb
@@ -93,4 +93,31 @@ describe 'Asciidoctor::PDF::Converter - Manpage' do
     (expect name_title_text[:font_size]).to be 22
     (expect pdf.lines).to include 'cmd - does stuff'
   end
+
+  it 'should arrange body of manpage into columns if specified in theme' do
+    pdf = to_pdf <<~'EOS', doctype: :manpage, pdf_theme: { page_columns: 2 }, analyze: true
+    = cmd(1)
+
+    == Name
+
+    cmd - does stuff
+
+    == Synopsis
+
+    *cmd* [_OPTION_]... _FILE_...
+
+    <<<
+
+    == Options
+
+    *-v*:: Prints the version.
+    EOS
+
+    midpoint = (get_page_size pdf)[0] * 0.5
+    name_text = pdf.find_unique_text 'Name'
+    options_text = pdf.find_unique_text 'Options'
+    (expect name_text[:x]).to eql 48.24
+    (expect options_text[:x]).to be > midpoint
+    (expect name_text[:y]).to eql options_text[:y]
+  end
 end
diff --git a/spec/page_spec.rb b/spec/page_spec.rb
index 62ec84e2..be80b9be 100644
--- a/spec/page_spec.rb
+++ b/spec/page_spec.rb
@@ -538,6 +538,177 @@ describe 'Asciidoctor::PDF::Converter - Page' do
     end
   end
 
+  context 'Columns' do
+    it 'should ignore columns for book doctype' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 2 }, analyze: true
+      = Document Title
+      :doctype: book
+      :notitle:
+
+      [.text-right]
+      first page
+
+      <<<
+
+      second page
+      EOS
+
+      midpoint = (get_page_size pdf)[0] * 0.5
+      (expect pdf.pages).to have_size 2
+      (expect (pdf.find_unique_text 'first page')[:page_number]).to eql 1
+      (expect (pdf.find_unique_text 'first page')[:x]).to be > midpoint
+      (expect (pdf.find_unique_text 'second page')[:page_number]).to eql 2
+    end
+
+    it 'should ignore columns if less than 2' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 1 }, analyze: true
+      = Document Title
+      :notitle:
+
+      first page
+
+      <<<
+
+      second page
+      EOS
+
+      (expect pdf.pages).to have_size 2
+      (expect (pdf.find_unique_text 'first page')[:page_number]).to eql 1
+      (expect (pdf.find_unique_text 'second page')[:page_number]).to eql 2
+    end
+
+    it 'should arrange article body into columns' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 2 }, analyze: true
+      first column
+
+      <<<
+
+      second column
+
+      <<<
+
+      [.text-right]
+      first column again
+      EOS
+
+      midpoint = (get_page_size pdf)[0] * 0.5
+      (expect pdf.pages).to have_size 2
+      (expect (pdf.find_unique_text 'first column')[:page_number]).to eql 1
+      (expect (pdf.find_unique_text 'second column')[:page_number]).to eql 1
+      (expect (pdf.find_unique_text 'second column')[:x]).to be > midpoint
+      (expect (pdf.find_unique_text 'first column again')[:page_number]).to eql 2
+      (expect (pdf.find_unique_text 'first column again')[:x]).to be < midpoint
+    end
+
+    it 'should put footnotes at bottom of last column with content' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 2 }, analyze: true
+      first columnfootnote:[This page has two columns.]
+
+      <<<
+
+      second column
+      EOS
+
+      midpoint = (get_page_size pdf)[0] * 0.5
+      (expect pdf.pages).to have_size 1
+      (expect (pdf.find_unique_text 'second column')[:x]).to be > midpoint
+      right_column_text = pdf.text.select {|it| it[:x] > midpoint }
+      right_column_lines = pdf.lines right_column_text
+      (expect right_column_lines).to have_size 2
+      (expect right_column_lines[-1]).to eql '[1] This page has two columns.'
+    end
+
+    it 'should place document title outside of column box' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 2 }, analyze: true
+      = Article Title Goes Here
+
+      first column
+
+      <<<
+
+      second column
+      EOS
+
+      midpoint = (get_page_size pdf)[0] * 0.5
+      (expect pdf.pages).to have_size 1
+      title_text = pdf.find_unique_text 'Article Title Goes Here'
+      (expect title_text[:x]).to be < midpoint
+      (expect title_text[:x] + title_text[:width]).to be > midpoint
+      (expect (pdf.find_unique_text 'second column')[:x]).to be > midpoint
+    end
+
+    it 'should place TOC outside of column box' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 2 }, analyze: true
+      = Article Title Goes Here
+      :toc:
+
+      == First Column
+
+      <<<
+
+      == Second Column
+      EOS
+
+      midpoint = (get_page_size pdf)[0] * 0.5
+      (expect pdf.pages).to have_size 1
+      first_column_text = (pdf.find_text 'First Column').sort_by {|it| -it[:y] }
+      second_column_text = (pdf.find_text 'Second Column').sort_by {|it| -it[:y] }
+      (expect first_column_text[0][:x]).to eql 48.24
+      (expect first_column_text[1][:x]).to eql 48.24
+      (expect second_column_text[0][:x]).to eql 48.24
+      (expect second_column_text[1][:x]).to be > midpoint
+      dots_text = pdf.text.select {|it| it[:string].include? '.' }
+      dots_text.each do |it|
+        (expect it[:x]).to be < midpoint
+        (expect it[:x] + it[:width]).to be > midpoint
+      end
+    end
+
+    it 'should allow theme to control number of columns' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 4 }, analyze: true
+      one
+
+      <<<
+
+      two
+
+      <<<
+
+      three
+
+      <<<<
+
+      four
+      EOS
+
+      midpoint = (get_page_size pdf)[0] * 0.5
+      (expect pdf.pages).to have_size 1
+      one_text = pdf.find_unique_text 'one'
+      two_text = pdf.find_unique_text 'two'
+      three_text = pdf.find_unique_text 'three'
+      four_text = pdf.find_unique_text 'four'
+      (expect two_text[:x]).to be > one_text[:x]
+      (expect two_text[:x]).to be < midpoint
+      (expect four_text[:x]).to be > three_text[:x]
+      (expect three_text[:x]).to be > midpoint
+    end
+
+    it 'should allow theme to control column gap' do
+      pdf = to_pdf <<~'EOS', pdf_theme: { page_columns: 2, page_column_gap: 12 }, analyze: :image
+      image::square.png[pdfwidth=100%]
+
+      <<<
+
+      image::square.png[pdfwidth=100%]
+      EOS
+
+      images = pdf.images
+      (expect images).to have_size 2
+      column_gap = (images[1][:x] - (images[0][:x] + images[0][:width])).to_f
+      (expect column_gap).to eql 12.0
+    end
+  end
+
   context 'Background' do
     it 'should set page background to white if value is not defined or transparent', visual: true do
       [nil, 'transparent'].each do |bg_color|