Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use ReVIEW::Converter instead of system("review-compile ...") #493

Merged
merged 2 commits into from Mar 17, 2016

Conversation

takahashim
Copy link
Collaborator

  • ReVIEW::Configure: add some default values
  • ReVIEW::EPUBMaker, ReVIEW::PDFMaker: basedir should be same as yamlfile
  • ReVIEW::EPUBMaker: stop building when error occured in compile *.re files.

review-epubmakerとreview-pdfmakerで、内部的にreview-compileを呼ぶのを止めて、compile用のReVIEW::Converterクラスを使うようにするものです。
review-epubmakerは5倍くらいは速くなりそうです(review-pdfmakerはLaTeX側処理が圧倒的に時間がかかっているのであんまり変わらない)。
ただし、review-compileは別コマンドを呼ぶことによってBookオブジェクト等が共有されていなかったのですが、ReVIEW::Converterでは積極的に共有する形にしているので、思わぬ結果になる可能性があります。

* ReVIEW::Configure: add some default values
* ReVIEW::EPUBMaker, ReVIEW::PDFMaker: basedir should be same as yamlfile
* ReVIEW::EPUBMaker: stop building when error occured in compile *.re files.
@takahashim
Copy link
Collaborator Author

それと、config.ymlのparams:属性が無視されます(paramsはreview-compileに渡すパラメータだったため)。一応paramsが設定されていた場合は警告を出すようにはしてますが、errorにはしていません(errorにして実行止めてもいいかも)。

@kmuto
Copy link
Owner

kmuto commented Mar 14, 2016

ありがとうございます。paramsは最近のだともう使ってることはないので大丈夫かな。

共有化による現象はちょっと怖いところですが、それで何か起きたら直すべきバグというのもわかるか…。

* remove `load_builder` method from ReVIEW::Converter.
* easy to pass parameters to Builder object.
takahashim added a commit that referenced this pull request Mar 17, 2016
use ReVIEW::Converter instead of system("review-compile ...")
@takahashim takahashim merged commit 9d9958d into master Mar 17, 2016
@takahashim
Copy link
Collaborator Author

とりいそぎマージしてみました

@takahashim takahashim deleted the converter branch March 17, 2016 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants