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

review-index: refine TOCParser and TOCPrinter #486

Merged
merged 12 commits into from
Feb 13, 2016
Merged

review-index: refine TOCParser and TOCPrinter #486

merged 12 commits into from
Feb 13, 2016

Conversation

takahashim
Copy link
Collaborator

目次周りを何とかするべく、TOCParserを大工事しています。

  • TOCRoot を廃止して、既存のReVIEW::Book::* のクラスに後からincludeする形の修正をなくし、TOCParserで定義しているクラス郡とは完全に別扱いするようになっています。
    • 現状はTOCParserをrequireするとReVIEW::Book::*にeach_sectionとかを追加して、上手い感じに扱えるようなハックが入っているのですが、ちょっとハック過ぎるというか、同じ物として扱おうとするのは筋が悪そうです。少なくともTOCParser::ChapterとReVIEW::Book::Chapterの両方がある時点でやばそう
  • TOCPrinter側も、TEXT/HTML/IDGで別々のロジックになっていたところを統一しています。
    • というかHTMLがまともに動いてなかった。たぶん--htmlオプションは誰も使ってない。
    • -aで文書全体、-pで部単位、-cで章単位に出力するところを、print_book, print_part, print_chapterのメソッドに切り分けて、各メソッドの実装もこれらのメソッドを使うようにしています。
  • ruby -wをつけて警告が出るところを一部修正しています。

今のところTOCParserはreview-indexコマンドでしか使っていないはずですが、本体でも使うようにする予定です。

@takahashim
Copy link
Collaborator Author

@kmuto --idgって使われてます? IDGTOCPrinterのFIXMEとコードが気になりました(ロジック的にはもうちょっとマシに直せるはずだけどIDGのXMLの仕様が分からない)。

@kmuto
Copy link
Owner

kmuto commented Feb 7, 2016

実際の利用シーンで使うことがなかったので、IDGTocのほうは削除してOKです。

@takahashim
Copy link
Collaborator Author

@kmuto なんと。それではIDGTOCPrinterの方は後で消しておきます

takahashim added a commit that referenced this pull request Feb 13, 2016
review-index: refine TOCParser and TOCPrinter
@takahashim takahashim merged commit aa968cd into master Feb 13, 2016
@takahashim takahashim deleted the toc-parser branch February 13, 2016 04:14
@kdmsnr kdmsnr added this to the 2.0.0 milestone Apr 29, 2016
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.

3 participants