add Catalog#validate! #957

takahashim · 2018-02-25T17:03:03Z

raise FileNotFound error when *.re file in catalog.yml does not exist

raise FileNotFound error when `*.re` file does not exist

takahashim · 2018-02-25T17:08:27Z

#955 とは違ってCatalogオブジェクトのチェックをするものですが、現状だと例外がキャッチされずにすごいことになるんですよね…。
とはいえ適当に拾うと他のエラーが抑制されるかもしれないので要注意です。

$ rake pdf
review-pdfmaker config.yml
Traceback (most recent call last):
	17: from /Users/maki/.rbenv/versions/2.5.0/bin/review-pdfmaker:23:in `<main>'
	16: from /Users/maki/.rbenv/versions/2.5.0/bin/review-pdfmaker:23:in `load'
	15: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/bin/review-pdfmaker:16:in `<top (required)>'
	14: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/pdfmaker.rb:81:in `execute'
	13: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/pdfmaker.rb:123:in `execute'
	12: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/pdfmaker.rb:217:in `generate_pdf'
	11: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/pdfmaker.rb:128:in `make_input_files'
	10: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/book/base.rb:102:in `parts'
	 9: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/book/base.rb:283:in `read_parts'
	 8: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/book/base.rb:303:in `parse_chapters'
	 7: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/book/base.rb:189:in `catalog'
	 6: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/catalog.rb:53:in `validate!'
	 5: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/catalog.rb:53:in `each'
	 4: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/catalog.rb:55:in `block in validate!'
	 3: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/catalog.rb:55:in `each'
	 2: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/catalog.rb:60:in `block (2 levels) in validate!'
	 1: from /Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/catalog.rb:60:in `each'
/Users/maki/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/review-2.4.0/lib/review/catalog.rb:62:in `block (3 levels) in validate!': file not found in catalog.yml: ./hoge2.re (ReVIEW::FileNotFound)
rake aborted!
Command failed with status (1): [review-pdfmaker config.yml...]
/Users/maki/tmp/hoge/Rakefile:59:in `block in <top (required)>'
Tasks: TOP => pdf => book.pdf
(See full trace by running task with --trace)

kmuto · 2018-02-25T23:24:27Z

review-compileのほうも手当てしたいので、そうなるとcompilerでやるほうがいいんじゃないかなー

takahashim · 2018-02-26T10:30:25Z

確かにこれだけでは足りない場合もありますが、

Catalogオブジェクトとしてはファイルが存在しない＝明らかに（静的に）おかしい状態をvalidateする機能は持っているべき
各*.reファイルのparseの前に確認できるので問題がある場合に早めに確認できるので便利

ということで、これはこれであった方がいいかと。

あとはどこかでFileNotFoundをrescueするのを追加します。

kmuto · 2018-02-26T10:38:25Z

了解です、review-compileのほうは別に持たせましょうか。

takahashim · 2018-02-27T04:43:33Z

FileNotFoundをあげるようにして、YAML errorを拾ってるのと同じ階層でrescueするようにしました。
$DEBUGが有効になってると、死んだ時にバックトレースが表示されるようになっています（が、これを有効にするにはruby -d -S review-pdfmakerみたいに実行する必要がありそう。config.ymlにdebug: trueしてるときに$DEBUGも有効にした方がいいのかな…）。

kmuto · 2018-02-27T04:56:55Z

raise if config['debug']のようなかんじだとダメですか

takahashim · 2018-02-27T06:11:05Z

そっちにしてみましょうか。

kmuto · 2018-02-27T06:37:55Z

ありがとうございます、問題なさそうに思います。
review-compileのほうはこんなかんじでしょうか。

diff --git a/bin/review-compile b/bin/review-compile
index d0ba754..5fcc77d 100755
--- a/bin/review-compile
+++ b/bin/review-compile
@@ -82,6 +82,7 @@ def _main
       book = ReVIEW::Book::Base.load(@basedir)
       book.config = @config # needs only at the first time
       ARGV.each do |item|
+        error("file not found: #{item}") unless File.exist?(item)
         chap_name = File.basename(item, '.*')
         chap = book.chapter(chap_name)
         compiler = ReVIEW::Compiler.new(load_strategy_class(@target, @check_only))

kmuto · 2018-02-27T06:41:09Z

あれ、なんか手元のドキュメントで過剰反応してコケますね、ちょっと検証します…

kmuto · 2018-02-27T07:04:41Z

なるほど、部構成に対応できてないっぽいです。

CHAPS:
 - part1.re:
   - ch01.re
   - ch02.re
 - PART2:
   - ch03.re
   - ch04.re

E, [2018-02-27T16:03:19.408117 #25641] ERROR -- : review-pdfmaker: file not found in catalog.yml: ./["ch01.re", "ch02.re"]

kmuto · 2018-02-27T07:14:32Z

filenames = item.values.flatten にすればいちおう通りそうだけれども、ハッシュキーである部のほうが問題ですね。

「部ファイルがあればそれを使う、なければ文字列とする」という実装になっているので、存在エラーとは相性が悪そう。

kmuto · 2018-02-27T07:18:55Z

拾った文字列が「拡張子.re」でかつファイル不存在なら「怪しい?」旨の警告を出す、というのはややこしくなりそうですね…

kdmsnr · 2018-02-27T07:48:35Z

メジャーバージョンを上げるなら、すべて *.re にする（文字列は使わない）に変更するのもありかなあと

kmuto · 2018-02-27T08:52:51Z

うーん、それは手元のプロジェクト群で死ぬものがけっこうありますね…

現状にflattenとreview-compileパッチを入れてマージしたいと思いますがいかがでしょう。@takahashim

takahashim · 2018-02-27T11:28:39Z

確かにpartがあると失敗してるので、気を取り直して実装を直しました

kmuto · 2018-02-27T12:10:59Z

マージします、ありがとうございました！

takahashim · 2018-02-27T12:23:12Z

あ、マージされた。えっと、これはこれとして、review-compileの方にも、

diff --git a/bin/review-compile b/bin/review-compile
index d0ba754..5fcc77d 100755
--- a/bin/review-compile
+++ b/bin/review-compile
@@ -82,6 +82,7 @@ def _main
       book = ReVIEW::Book::Base.load(@basedir)
       book.config = @config # needs only at the first time
       ARGV.each do |item|
+        error("file not found: #{item}") unless File.exist?(item)
         chap_name = File.basename(item, '.*')
         chap = book.chapter(chap_name)
         compiler = ReVIEW::Compiler.new(load_strategy_class(@target, @check_only))

的なチェックは入れておいた方がよいですか？

kmuto · 2018-02-27T13:18:00Z

ちょっと急ぎすぎたか、すいません。
はい、このブランチでreview-compileのほうに上記のチェックは入れています。934d274

add Catalog#validate!

189f22f

raise FileNotFound error when `*.re` file does not exist

takahashim added 3 commits February 27, 2018 13:06

handle Errors in *maker

ee35e41

Merge branch 'master' into catalog_validate

ac088e5

move rescue blocks to avoid exiting from deeper methods

2013d50

use @config["debug"] instead of $DEBUG

6b7753c

kdmsnr mentioned this pull request Feb 27, 2018

Convert old part style into new style #965

Merged

support parts

04e7d54

check file existence in review-compile.

934d274

kmuto merged commit 83ac1b5 into master Feb 27, 2018

kmuto deleted the catalog_validate branch February 27, 2018 12:11

This was referenced Feb 27, 2018

raise an error when file not found. Closes #953 #955

Closed

原稿ファイルが存在しないときのエラーメッセージがとても分かりにくい #953

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add Catalog#validate! #957

add Catalog#validate! #957

takahashim commented Feb 25, 2018

takahashim commented Feb 25, 2018

kmuto commented Feb 25, 2018

takahashim commented Feb 26, 2018

kmuto commented Feb 26, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kdmsnr commented Feb 27, 2018

kmuto commented Feb 27, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018

add Catalog#validate! #957

add Catalog#validate! #957

Conversation

takahashim commented Feb 25, 2018

takahashim commented Feb 25, 2018

kmuto commented Feb 25, 2018

takahashim commented Feb 26, 2018

kmuto commented Feb 26, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kmuto commented Feb 27, 2018

kdmsnr commented Feb 27, 2018

kmuto commented Feb 27, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018

takahashim commented Feb 27, 2018

kmuto commented Feb 27, 2018