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

feat: Support multiple -o and -f options #96

Merged
merged 14 commits into from
Jan 14, 2021

Conversation

spring-raining
Copy link
Member

@spring-raining spring-raining commented Jan 6, 2021

Overview

This PR makes the CLI acceptable multiple -o and -f options. See #86 for the detail.

Obsoleted options

  • --out-file
  • --out-dir
  • --dist-dir

@spring-raining spring-raining marked this pull request as ready for review January 10, 2021 20:14
@MurakamiShinyu
Copy link
Member

MurakamiShinyu commented Jan 11, 2021

出力先を指定しないと "Error: Please specify output option(s)." になります。

出力先指定を必須にすることにしたのであれば、--help で表示される次の記述、

  -o, --output <path>           specify output file name or directory [<title>.pdf]

[<title>.pdf] の記述を削除する必要がありますね。

また、CONTRIBUTING.md のとおりに example の build をすると、出力先の指定がないのでこのエラーになります。

@spring-raining
Copy link
Member Author

そうでした、従来の実装通りvivliostyle.config.jsコンテキスト内であれば <title>.pdf 、そうでなければ output.pdf で出力するようにします。

@MurakamiShinyu
Copy link
Member

webbook の出力先が指定できるようになりましたが、その動作は、
temporary directory に常に webbook のようなもの(以前のv3.0-preの distDir に出力されていたもの)を作り
webbook 出力の指定があれば、temporary directory から webbook 出力先にコピーする、
というものですね。

気になるところ:

  • MarkdownからのCSS組版でPDFを作る場合の中間形式であるHTMLファイル群がユーザーからは見えない tmp dir に作られて実行後に自動削除されることについて
    • メリットは、ユーザーにとって不要な中間ファイルが作業ディレクトリ内に残らないこと
    • デメリットは、Vivliostyle Core が組版処理をするHTMLファイル群が見えないところにできて消えてしまうので、HTML+CSSの問題を見つけにくく、デバッグがしづらい
  • webbook 出力先の指定がある場合には tmp dir に作ってからコピーするのではなく、はじめからその出力先に出力したほうがよいのでないか?
    • 原稿を変更したらすぐに webbook ディレクトリ内が更新されるwatchモードがほしい。その場合はじめからwebbook ディレクトリに出力する必要があるはず
  • webbook 出力先が input のルート(通常カレントディレクトリ)と同じ場合には、markdown と同じところに html を生成して、画像ファイルなどのコピーをしないようになることを期待。

@spring-raining
Copy link
Member Author

MarkdownからのCSS組版でPDFを作る場合の中間形式であるHTMLファイル群がユーザーからは見えない tmp dir に作られて実行後に自動削除されることについて

temporary directoryを作る現在の実装は、.vivliostyle を作らないようにするための暫定的な対応です。変更が大きすぎるためこのPRでは対応しない予定でしたが、最終的にはin-placeでトランスパイルを実行するような処理に変える必要があるでしょう。その対応の際に、同じディレクトリにhtmlを保存する、などの対応をするのが良さそうです。

webbook 出力先の指定がある場合には tmp dir に作ってからコピーするのではなく、はじめからその出力先に出力したほうがよいのでないか?

確かにその通りではありますが、若干考慮すべき点が増えることを懸念しています (-f webbook の出力を複数指定された際はどうするか? 等)。watchモードを実装する際も、変更される都度一旦変換結果をtmp dirに保存してそれをコピーする実装で問題ないと思います。

@MurakamiShinyu
Copy link
Member

簡単なテストをいくつかしました。

まずテスト用の単純なMarkdownファイルを作る:

$ echo '# Hello World' > example.md

ビルド実行:

$ vivliostyle build example.md 
✖ Error: title not defined

If you think this is a bug, please report at https://github.com/vivliostyle/vivliostyle-cli/issues

これはこのバグ: #61 ですね。

そこで --title 指定:

$ vivliostyle build example.md --title Test
◡ example.md Hello WorldwaitFor is deprecated and will be removed in a future release. See https://github.com/puppeteer/puppeteer/issues/6214 for details and how to migrate your code.
✔ example.md Hello World
✔ example.md Hello World
◡ Building PDF
output.pdf has been created.
🎉 Built successfully.

OK。デフォルトの出力PDF "output.pdf" できた。

次に、出力先PDFファイルのパスを指定:

$ vivliostyle build example.md --title Test -o outdir/pdf1.pdf
◡ example.md Hello WorldwaitFor is deprecated and will be removed in a future release. See https://github.com/puppeteer/puppeteer/issues/6214 for details and how to migrate your code.
✔ example.md Hello World
✔ example.md Hello World
◡ Building PDF
outdir/pdf1.pdf has been created.
🎉 Built successfully.

OK。

次に、出力先として.pdf拡張子付きのパス、同時に -f webbook を指定してテスト:

$ vivliostyle build example.md --title Test -o outdir/isThisPDF.pdf -f webbook
◡ Collecting build config
outdir/isThisPDF.pdf has been created.
🎉 Built successfully.

"outdir/isThisPDF.pdf has been created." と表示されるが、出力されたものはPDFファイルではなく webbook であった。拡張子からの推定よりも -f の指定を優先していることが分かる。

では -f webbook を指定して、 出力先 -o を指定しない場合はどうなるかテスト:

$ vivliostyle build example.md --title Test -f webbook
◡ example.md Hello WorldwaitFor is deprecated and will be removed in a future release. See https://github.com/puppeteer/puppeteer/issues/6214 for details and how to migrate your code.
✔ example.md Hello World
✔ example.md Hello World
◡ Building PDF
output.pdf has been created.
🎉 Built successfully.

この場合は PDF ができた。-f の指定が優先とも限らないよう。
-f の指定を優先する方針ならばデフォルトの出力先に webbook を出力するべきだろうが、inputと同じところにwebbookを作る機能がまだできないので、この動作は暫定的か?

では、-f の指定なしで、拡張子なしの出力先の場合はどうか。

$ vivliostyle build example.md --title Test -o output2
◡ Collecting build config
output2 has been created.
🎉 Built successfully.

拡張子がない出力先指定は webbook の出力になった。拡張子なし(あるいは不明な拡張子)の指定の場合は webbook と推定ということか。

では、-f pdf の指定つきで、拡張子なしの出力先の場合 -o output3 はどうか。拡張子なしのPDFファイルができるか、それとも指定の名前のディレクトリにデフォルトの名前でPDFができる("output3/output.pdf" になる)か?

$ vivliostyle build example.md --title Test -o output3 -f pdf
◡ example.md Hello WorldwaitFor is deprecated and will be removed in a future release. See https://github.com/puppeteer/puppeteer/issues/6214 for details and how to migrate your code.
✔ example.md Hello World
✔ example.md Hello World
◡ Building PDF
output3 has been created.
🎉 Built successfully.

拡張子なしのPDFファイル "output3" ができた。指定の名前のディレクトリにデフォルトの名前でPDFができるのではなかった。

では同じく -f pdf の指定つきで、出力先がディレクトリであることが分かるように名前のあとにスラッシュをつけた出力先の指定 -o output4/ にした場合はどうか:

$ vivliostyle build example.md --title Test -o output4/ -f pdf
◠ example.md Hello WorldwaitFor is deprecated and will be removed in a future release. See https://github.com/puppeteer/puppeteer/issues/6214 for details and how to migrate your code.
✔ example.md Hello World
✔ example.md Hello World
◡ Building PDF
output4 has been created.
🎉 Built successfully.

名前のあとにつけたスラッシュは意味がなかったようで、拡張子なしのPDFファイル "output4" ができた。

すでに存在するディレクトリを出力先に指定した場合はどうかテスト:

$ mkdir output5
$ vivliostyle build example.md --title Test -o output5/ -f pdf
◠ example.md Hello WorldwaitFor is deprecated and will be removed in a future release. See https://github.com/puppeteer/puppeteer/issues/6214 for details and how to migrate your code.
✔ example.md Hello World
✔ example.md Hello World
✖ Error: EISDIR: illegal operation on a directory, open '/private/tmp/work/output5'

If you think this is a bug, please report at https://github.com/vivliostyle/vivliostyle-cli/issues

すでに存在するディレクトリ名を出力先にしてのPDF出力はできず、エラーに。PDFファイルの出力先の指定はディレクトリだけの指定は不可で必ずファイル名が必要ということで一貫性はある。

次に、-f webbook で、すでに存在するディレクトリを出力先に指定した場合をテスト:

$ vivliostyle build example.md --title Test -o output5/ -f webbook
◡ Collecting build config
output5 has been created.
🎉 Built successfully.

"output5" ディレクトリに webbook ができたよう。しかし、それを見てみると:

$ ls -laF output5/
total 0
drwxr-xr-x  4 shinyu  wheel  128 Jan 12 14:44 ./
drwxr-xr-x  9 shinyu  wheel  288 Jan 12 12:25 ../
drwx------  5 shinyu  wheel  160 Jan 12 14:43 tmp-27905-2t2X2JcODvW8/

指定したディレクトリ "output5" の中に別のディレクトリ "tmp-27905-2t2X2JcODvW8" ができている。
tmp dir からのディレクトリのコピーに問題があるよう。
#94 と同様の修正が必要)

@spring-raining
Copy link
Member Author

上記のテストのうち

vivliostyle build example.md --title Test -f webbook
vivliostyle build example.md --title Test -o output5/ -f webbook

の結果は望ましくないため修正しました。-o なしで -f を設定した場合エラーとなるようにしています。

@spring-raining spring-raining merged commit c3665e7 into develop Jan 14, 2021
@spring-raining spring-raining deleted the feat/multiple-build-targets branch January 14, 2021 11:36
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