From e3c3fb99a409a187adc41320f44b444e19663dd1 Mon Sep 17 00:00:00 2001 From: takahashim Date: Sun, 18 Aug 2019 16:30:05 +0900 Subject: [PATCH 1/2] Catalog#{chaps,parts,predef,postdef,appendix} should return Array, not String --- lib/review/book/base.rb | 16 ++++++------ lib/review/catalog.rb | 23 ++++++++--------- test/test_book.rb | 8 +++--- test/test_catalog.rb | 56 ++++++++++++----------------------------- 4 files changed, 38 insertions(+), 65 deletions(-) diff --git a/lib/review/book/base.rb b/lib/review/book/base.rb index 582f5c35d..93638bcfe 100644 --- a/lib/review/book/base.rb +++ b/lib/review/book/base.rb @@ -200,7 +200,7 @@ def catalog def read_chaps if catalog - catalog.chaps + catalog.chaps.join("\n") else read_file(config['chapter_file']) end @@ -208,7 +208,7 @@ def read_chaps def read_predef if catalog - catalog.predef + catalog.predef.join("\n") else read_file(config['predef_file']) end @@ -216,7 +216,7 @@ def read_predef def read_appendix if catalog - catalog.appendix + catalog.appendix.join("\n") else read_file(config['postdef_file']) # for backward compatibility end @@ -224,7 +224,7 @@ def read_appendix def read_postdef if catalog - catalog.postdef + catalog.postdef.join("\n") else '' end @@ -234,7 +234,7 @@ def read_part return @read_part if @read_part if catalog - @read_part = catalog.parts + @read_part = catalog.parts.join("\n") else @read_part = File.read(File.join(@basedir, config['part_file'])) end @@ -258,7 +258,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 +273,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 +290,7 @@ def appendix def postscripts if catalog - mkpart_from_namelist(catalog.postdef.split("\n")) + mkpart_from_namelist(catalog.postdef) end end diff --git a/lib/review/catalog.rb b/lib/review/catalog.rb index b74afbdb9..9bdb3a9fa 100644 --- a/lib/review/catalog.rb +++ b/lib/review/catalog.rb @@ -12,12 +12,11 @@ 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) @@ -25,11 +24,11 @@ def chaps 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) @@ -37,7 +36,7 @@ def parts 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) diff --git a/test/test_book.rb b/test/test_book.rb index 97a18d6b6..102e3cfdf 100644 --- a/test/test_book.rb +++ b/test/test_book.rb @@ -248,15 +248,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 } diff --git a/test/test_catalog.rb b/test/test_catalog.rb index 98e277cf8..b27c4694f 100644 --- a/test/test_catalog.rb +++ b/test/test_catalog.rb @@ -7,61 +7,43 @@ class CatalogTest < Test::Unit::TestCase def test_predef sut = Catalog.new(yaml) - exp = <<-EOS -pre01.re -pre02.re - EOS - assert_equal(exp.chomp, sut.predef) + exp = %w[pre01.re pre02.re] + assert_equal(exp, sut.predef) end def test_chaps sut = Catalog.new(yaml) - exp = <<-EOS -ch01.re -ch02.re - EOS - assert_equal(exp.chomp, sut.chaps) + exp = %w[ch01.re ch02.re] + assert_equal(exp, sut.chaps) end def test_chaps_empty yaml = StringIO.new sut = Catalog.new(yaml) - assert_equal('', sut.chaps) + assert_equal([], sut.chaps) end def test_appendix sut = Catalog.new(yaml) - exp = <<-EOS -post01.re -post02.re - EOS - assert_equal(exp.chomp, sut.appendix) + exp = %w[post01.re post02.re] + assert_equal(exp, sut.appendix) end def test_chaps_with_parts sut = Catalog.new(yaml_with_parts) - exp = <<-EOS -ch01.re -ch02.re -ch03.re -ch04.re -ch05.re - EOS - assert_equal(exp.chomp, sut.chaps) + exp = %w[ch01.re ch02.re ch03.re ch04.re ch05.re] + assert_equal(exp, sut.chaps) end def test_parts sut = Catalog.new(yaml_with_parts) - exp = <<-EOS -part1.re -part2.re - EOS - assert_equal(exp.chomp, sut.parts) + exp = %w[part1.re part2.re] + assert_equal(exp, sut.parts) end def test_parts_with_empty sut = Catalog.new(yaml) - assert_equal('', sut.parts) + assert_equal([], sut.parts) end def test_empty_parts @@ -80,20 +62,14 @@ def test_parts2 def test_postdef sut = Catalog.new(yaml) - exp = <<-EOS -back01.re -back02.re - EOS - assert_equal(exp.chomp, sut.postdef) + exp = %w[back01.re back02.re] + assert_equal(exp, sut.postdef) end def test_from_object sut = Catalog.new(yaml_hash) - exp = <<-EOS -ch01.re -ch02.re - EOS - assert_equal(exp.chomp, sut.chaps) + exp = %w[ch01.re ch02.re] + assert_equal(exp, sut.chaps) end def test_validate From 53502f415e00d088604ca8ee81b67f85fa43f133 Mon Sep 17 00:00:00 2001 From: takahashim Date: Sun, 18 Aug 2019 18:44:38 +0900 Subject: [PATCH 2/2] fix read_*, remove cache @read_part --- lib/review/book/base.rb | 32 ++++++++++++++------------------ lib/review/book/chapter.rb | 2 +- test/test_book.rb | 31 ++++++++++++++++--------------- 3 files changed, 31 insertions(+), 34 deletions(-) diff --git a/lib/review/book/base.rb b/lib/review/book/base.rb index 93638bcfe..eb64e0b89 100644 --- a/lib/review/book/base.rb +++ b/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 @@ -200,43 +199,41 @@ def catalog def read_chaps if catalog - catalog.chaps.join("\n") + catalog.chaps else - read_file(config['chapter_file']) + read_file(config['chapter_file']).split("\n") end end def read_predef if catalog - catalog.predef.join("\n") + catalog.predef else - read_file(config['predef_file']) + read_file(config['predef_file']).split("\n") end end def read_appendix if catalog - catalog.appendix.join("\n") + 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.join("\n") + catalog.postdef else - '' + [] end end def read_part - return @read_part if @read_part - if catalog - @read_part = catalog.parts.join("\n") + 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 @@ -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 diff --git a/lib/review/book/chapter.rb b/lib/review/book/chapter.rb index 6f472042d..072c672fb 100644 --- a/lib/review/book/chapter.rb +++ b/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 diff --git a/test/test_book.rb b/test/test_book.rb index 102e3cfdf..a8bb52fa2 100644 --- a/test/test_book.rb +++ b/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