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

EPUBMaker: verify_target_imagesが有効なときに、coverimageを暗黙に取り込む #1923

Merged
merged 13 commits into from
Aug 19, 2024

Conversation

kmuto
Copy link
Owner

@kmuto kmuto commented Aug 16, 2024

#1918 で言及した、verify_target_imagesが有効なときにcoverimageの画像ファイルも暗黙に取り込むものです。
imagedirで設定されたフォルダ(通常はimages)直下にあるcoverimageの画像ファイルを探す決めうちとしています。

@kmuto kmuto changed the title include coverimage file implicitly when verify_target_images is enabl… EPUBMaker: verify_target_imagesが有効なときに、coverimageを暗黙に取り込む Aug 16, 2024
@kmuto kmuto marked this pull request as draft August 16, 2024 13:50
@kmuto kmuto marked this pull request as ready for review August 18, 2024 05:42
@kmuto
Copy link
Owner Author

kmuto commented Aug 18, 2024

Windows固有の挙動に釈然としていないのですが、テンポラリフォルダ削除時のEACCES, ENOTEMPTYを握り潰すようにしています。
テストのみなので、実行への影響はありません。

@kmuto kmuto requested a review from takahashim August 18, 2024 12:51
Copy link
Collaborator

@takahashim takahashim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます、良いと思います!

@takahashim
Copy link
Collaborator

細かいところで気になるところはあるんですが、このPRとはあんまり関係なさそうなのでこれはこれで取り込んでいいんではないかと(例えば\Z\zにしたい気がするんですが、全体的に\Zが多用されてるんで別途見直した方がよさそうとか)。

@kmuto
Copy link
Owner Author

kmuto commented Aug 19, 2024

細かいところで気になるところはあるんですが、このPRとはあんまり関係なさそうなのでこれはこれで取り込んでいいんではないかと(例えば\Zは\zにしたい気がするんですが、全体的に\Zが多用されてるんで別途見直した方がよさそうとか)。

はい、そのあたりは全体ガッツリやっちゃってください!

@kmuto kmuto merged commit f143153 into master Aug 19, 2024
17 checks passed
@kmuto kmuto deleted the include-coverimage branch August 19, 2024 12:06
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