momijizukamori / bookbinder-js

A JS application to format PDFs for bookbinding.
Mozilla Public License 2.0
99 stars 26 forks source link

Replace perfectbound logic with sigsize = 1 #85

Closed acestronautical closed 4 months ago

acestronautical commented 4 months ago

It seems like we might be able to remove all the perfect bound logic, and instead treat perfectbound as signature size of 1.

This would simply the codebase if true.

github-actions[bot] commented 4 months ago

PR Preview Action v1.4.7 :---: Preview removed because the pull request was closed. 2024-02-12 05:47 UTC

acestronautical commented 4 months ago

Testing this against main the PDFs generating for perfectbound seem identical.

acestronautical commented 4 months ago

I rebased this and fixed the problem where it would allow generating signature files. if perfectbound or booklet is selected then only the aggregate file is generated.

momijizukamori commented 4 months ago

Two relatively small things: 1) when the setting is 'perfectbound', we should probably not display signature breakdown info, particularly because for longer docs that's going to get messy fast (current main branch implementation displays 'N/A' for perfectbound) 2) Can we move and rename the tests, rather than deleting them? May need one or two other tweaks to work with the new flow, but I don't think that should be hard.

acestronautical commented 4 months ago

I can rewrite the test but I would prefer to do that in a different PR since they will basically be new tests. Making the signature breakdown not display is a little tricky, even for very long books the list doesn't become unusably long, it's just a little ugly. I would prefer to push this as is and do the cleanup later if that's ok since this whole thing is about improving the codebase. Just not sure I have the time at the moment to do everything.

momijizukamori commented 4 months ago

Yeah that's fair - could you make placeholder issues for those two things, and I will approve this? I am a bit scatterbrained this weekend unfortunately, lol

acestronautical commented 4 months ago

Filed: https://github.com/momijizukamori/bookbinder-js/issues/91 https://github.com/momijizukamori/bookbinder-js/issues/92

😄 📖