Merge pull request #1372 from kmuto/fix-catalog

refactor Catalog

lib/review/book/base.rb

@@ -29,7 +29,6 @@ def initialize(basedir = '.')
         @chapter_index = nil
         @config = ReVIEW::Configure.values
         @catalog = nil
-        @read_part = nil
         @warn_old_files = {} # XXX for checking CHAPS, PREDEF, POSTDEF
         @basedir_seen = {}
         update_rubyenv
@@ -202,41 +201,39 @@ def read_chaps
         if catalog
           catalog.chaps
         else
-          read_file(config['chapter_file'])
+          read_file(config['chapter_file']).split("\n")
         end
       end
 
       def read_predef
         if catalog
           catalog.predef
         else
-          read_file(config['predef_file'])
+          read_file(config['predef_file']).split("\n")
         end
       end
 
       def read_appendix
         if catalog
           catalog.appendix
         else
-          read_file(config['postdef_file']) # for backward compatibility
+          read_file(config['postdef_file']).split("\n") # for backward compatibility
         end
       end
 
       def read_postdef
         if catalog
           catalog.postdef
         else
-          ''
+          []
         end
       end
 
       def read_part
-        return @read_part if @read_part
-
         if catalog
-          @read_part = catalog.parts
+          catalog.parts
         else
-          @read_part = File.read(File.join(@basedir, config['part_file']))
+          File.read(File.join(@basedir, config['part_file'])).split("\n")
         end
       end
 
@@ -258,7 +255,7 @@ def bib_exist?
 
       def prefaces
         if catalog
-          return mkpart_from_namelist(catalog.predef.split("\n"))
+          return mkpart_from_namelist(catalog.predef)
         end
 
         begin
@@ -273,7 +270,7 @@ def prefaces
 
       def appendix
         if catalog
-          names = catalog.appendix.split("\n")
+          names = catalog.appendix
           chaps = names.each_with_index.map { |n, idx| mkchap_ifexist(n, idx) }.compact
           return mkpart(chaps)
         end
@@ -290,7 +287,7 @@ def appendix
 
       def postscripts
         if catalog
-          mkpart_from_namelist(catalog.postdef.split("\n"))
+          mkpart_from_namelist(catalog.postdef)
         end
       end
 
@@ -324,7 +321,7 @@ def parse_chapters
                 chap = Chapter.new(self, num += 1, chap, File.join(contentdir, chap))
                 chap
               end
-              Part.new(self, part += 1, chaps, read_part.split("\n")[part - 1])
+              Part.new(self, part += 1, chaps, read_part[part - 1])
             else
               chap = Chapter.new(self, num += 1, entry, File.join(contentdir, entry))
               if chap.number
@@ -337,12 +334,11 @@ def parse_chapters
           end
         end
 
-        chap = read_chaps.
-               strip.lines.map(&:strip).join("\n").split(/\n{2,}/).
+        chap = read_chaps.map(&:strip).join("\n").split(/\n{2,}/).
                map do |part_chunk|
           chaps = part_chunk.split.map { |chapid| Chapter.new(self, num += 1, chapid, File.join(contentdir, chapid)) }
-          if part_exist? && read_part.split("\n").size > part
-            Part.new(self, part += 1, chaps, read_part.split("\n")[part - 1])
+          if part_exist? && read_part.size > part
+            Part.new(self, part += 1, chaps, read_part[part - 1])
           else
             Part.new(self, nil, chaps)
           end

lib/review/book/chapter.rb

@@ -125,7 +125,7 @@ def on_postdef?
       private
 
       def on_file?(contents)
-        contents.lines.map(&:strip).include?("#{id}#{@book.ext}")
+        contents.map(&:strip).include?("#{id}#{@book.ext}")
       end
 
       # backward compatibility

lib/review/catalog.rb

@@ -12,32 +12,31 @@ def initialize(file)
     end
 
     def predef
-      return '' unless @yaml['PREDEF']
-      @yaml['PREDEF'].join("\n")
+      @yaml['PREDEF'] || []
     end
 
     def chaps
-      return '' unless @yaml['CHAPS']
+      return [] unless @yaml['CHAPS']
 
       @yaml['CHAPS'].map do |entry|
         if entry.is_a?(String)
           entry
         elsif entry.is_a?(Hash)
           entry.values # chaps in a part
         end
-      end.flatten.join("\n")
+      end.flatten
     end
 
     def parts
-      return '' unless @yaml['CHAPS']
+      return [] unless @yaml['CHAPS']
 
       part_list = @yaml['CHAPS'].map do |entry|
         if entry.is_a?(Hash)
           entry.keys
         end
       end
 
-      part_list.flatten.compact.join("\n")
+      part_list.flatten.compact
     end
 
     def replace_part(old_name, new_name)
@@ -55,13 +54,11 @@ def parts_with_chaps
     end
 
     def appendix
-      return '' unless @yaml['APPENDIX']
-      @yaml['APPENDIX'].join("\n")
+      @yaml['APPENDIX'] || []
     end
 
     def postdef
-      return '' unless @yaml['POSTDEF']
-      @yaml['POSTDEF'].join("\n")
+      @yaml['POSTDEF'] || []
     end
 
     def to_s
@@ -71,7 +68,7 @@ def to_s
     def validate!(config, basedir)
       filenames = []
       if predef.present?
-        filenames.concat(predef.split(/\n/))
+        filenames.concat(predef)
       end
       parts_with_chaps.each do |chap|
         if chap.is_a?(Hash)
@@ -86,10 +83,10 @@ def validate!(config, basedir)
         end
       end
       if appendix.present?
-        filenames.concat(appendix.split(/\n/))
+        filenames.concat(appendix)
       end
       if postdef.present?
-        filenames.concat(postdef.split(/\n/))
+        filenames.concat(postdef)
       end
       filenames.each do |filename|
         refile = File.join(basedir, config['contentdir'], filename)

test/test_book.rb

@@ -32,7 +32,7 @@ def test_ext
   def test_read_chaps
     Dir.mktmpdir do |dir|
       book = Book::Base.new(dir)
-      assert_equal '', book.read_chaps
+      assert_equal [], book.read_chaps
 
       chaps_path = File.join(dir, 'CHAPS')
       re1_path = File.join(dir, "123#{book.ext}")
@@ -42,16 +42,16 @@ def test_read_chaps
       File.open(re1_path, 'w') { |o| o.print "123\n" }
       File.open(re2_path, 'w') { |o| o.print "456\n" }
 
-      assert_equal "abc\n", book.read_chaps
+      assert_equal ['abc'], book.read_chaps
 
       File.unlink(chaps_path)
-      assert_equal "#{re1_path}\n#{re2_path}", book.read_chaps
+      assert_equal [re1_path, re2_path], book.read_chaps
 
       File.unlink(re1_path)
-      assert_equal re2_path, book.read_chaps
+      assert_equal [re2_path], book.read_chaps
 
       File.unlink(re2_path)
-      assert_equal '', book.read_chaps
+      assert_equal [], book.read_chaps
     end
   end
 
@@ -68,17 +68,18 @@ def test_read_part
       File.open(chaps_path, 'w') { |o| o.print chaps_content }
 
       assert book.part_exist?
-      assert_equal chaps_content, book.read_part
+      assert_equal %w[abc], book.read_part
 
+      ## do not cache PART data
       File.open(chaps_path, 'w') { |o| o.print "XYZ\n" }
-      assert_equal chaps_content, book.read_part
+      assert_equal %w[XYZ], book.read_part
     end
   end
 
   def test_read_appendix
     Dir.mktmpdir do |dir|
       book = Book::Base.new(dir)
-      assert_equal '', book.read_appendix
+      assert_equal [], book.read_appendix
 
       post_path = File.join(dir, 'POSTDEF')
       re1_path = File.join(dir, "123#{book.ext}")
@@ -88,23 +89,23 @@ def test_read_appendix
       File.open(re1_path, 'w') { |o| o.print "123\n" }
       File.open(re2_path, 'w') { |o| o.print "456\n" }
 
-      assert_equal "abc\n", book.read_appendix
+      assert_equal ['abc'], book.read_appendix
 
       File.unlink(post_path)
-      assert_equal "#{re1_path}\n#{re2_path}", book.read_appendix
+      assert_equal [re1_path, re2_path], book.read_appendix
 
       File.unlink(re1_path)
-      assert_equal re2_path, book.read_appendix
+      assert_equal [re2_path], book.read_appendix
 
       File.unlink(re2_path)
-      assert_equal '', book.read_appendix
+      assert_equal [], book.read_appendix
     end
   end
 
   def test_read_postdef
     Dir.mktmpdir do |dir|
       book = Book::Base.new(dir)
-      assert_equal '', book.read_postdef
+      assert_equal [], book.read_postdef
 
       post_path = File.join(dir, 'POSTDEF')
       re1_path = File.join(dir, "123#{book.ext}")
@@ -114,10 +115,10 @@ def test_read_postdef
       File.open(re1_path, 'w') { |o| o.print "123\n" }
       File.open(re2_path, 'w') { |o| o.print "456\n" }
 
-      assert_equal '', book.read_postdef
+      assert_equal [], book.read_postdef
 
       File.unlink(post_path)
-      assert_equal '', book.read_postdef
+      assert_equal [], book.read_postdef
     end
   end
 
@@ -248,15 +249,15 @@ def test_parse_chpaters_with_parts_file
           ''
         ]
       ]
-    ].each do |n_parts, chaps_text, parts_text, part_names|
+    ].each do |n_parts, chaps, parts, part_names|
       n_test += 1
       Dir.mktmpdir do |dir|
         book = Book::Base.new(dir)
         chaps_path = File.join(dir, 'CHAPS')
-        File.open(chaps_path, 'w') { |o| o.print chaps_text }
-        if parts_text
+        File.open(chaps_path, 'w') { |o| o.print chaps }
+        if parts
           parts_path = File.join(dir, 'PART')
-          File.open(parts_path, 'w') { |o| o.print parts_text }
+          File.open(parts_path, 'w') { |o| o.print parts }
         end
 
         parts = book.instance_eval { parse_chapters }