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

Refactor EPUBMaker #1640

Merged
merged 18 commits into from
Jan 17, 2021
Merged

Refactor EPUBMaker #1640

merged 18 commits into from
Jan 17, 2021

Conversation

takahashim
Copy link
Collaborator

@takahashim takahashim commented Jan 9, 2021

#1575 に関連して、独立したライブラリではなくなったということでEPUBMaker自体を整理します。

  • ProducerEPUBv(2|3)(+EPUBCommon)の役割を整理する

ProducerEPUBv(2|3)との結合が強すぎるので、メソッドを移動して整理します。

大まかな分担としては、Producerはいわゆるファサード的な、「外から呼び出してEPUBを作る」ということに特化させて、実際にEPUB用の個別ファイルを生成する機能はEPUBv(2|3)に寄せるようにします。

最終的には、EPUBv(2|3)には@producerは不要で、@config@contentsだけProducerから渡すのでも十分なはずです。
@producerを消しました。結果、Producerとの相互依存ではなく、Producerから一方的に依存してるだけになりました。

  • EPUBv(2|3)内の@producer.configconfigにする 9220d97

@producerは極力使わないように。

  • mimetype等のメソッドにIOを渡すのはやめてStringを返すようにする 96497ae
  • mimetype等のメソッドはProducerではなくEPUBv(2|3)EPUBCommonに移動する bd18b63

上記2つが混沌としがちな原因だったので、ここを切り分けます。

  • EPUBCommon等で@producer.res.vを呼ぶのを止め、直接ReVIEW::I18n.tを呼ぶようにする cf6def8

Re:VIEWへの依存度を極力下げる必要がなければ、普通に書いた方が素直なはずです。

  • EPUBv(2|3)で定義されるメソッドをEPUBCommonでもNotImplementedで定義しておく bd18b63

なくても困らないですが、分かりやすさのため。

  • EPUBv2でしか使われないEPUBCommonのメソッドをEPUBv2に移動する 2587023

主にncx_*メソッドを対象としています。

@takahashim
Copy link
Collaborator Author

EPUBv(2|3)内のXML部分のテンプレート化も考えているのですが、更に修正が大きくなりそうなのでいったんここまでにします。

@takahashim takahashim requested a review from kmuto January 10, 2021 03:49
@kmuto
Copy link
Owner

kmuto commented Jan 10, 2021

ありがとうございます、今週中には手元データでいろいろ比較しておきます。
EPUBv2はもう捨ててもいいとは思っていますが(必要ならreview-2あたりを使う)、捨てると次バージョンがメジャーアップデート扱いになっちゃうので維持ですかね。

@takahashim takahashim mentioned this pull request Jan 11, 2021
@takahashim
Copy link
Collaborator Author

上に書いた「XML部分のテンプレート化」を #1641 にしてみました。試してもらうなら #1641 の方が良いかもです。

EPUBv2はこれ以上機能追加を考えなければそんなに複雑化しているわけではないので、急いで捨てる必要はないかと思いますが、warning出すとかはしてもよいかもです(hierarchy_ncxは複雑だけどEPUBv3でも使っているので捨てられないですし…)。

@takahashim
Copy link
Collaborator Author

明らかな問題なさそうなのでこれでマージしてみます。

@takahashim takahashim merged commit ac5f0ac into master Jan 17, 2021
@takahashim takahashim deleted the refactor-epubmaker branch January 17, 2021 07:01
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.

2 participants