gvellut / jncep

Command-line tool to generate EPUB files for J-Novel Club pre-pub novels
GNU General Public License v3.0
43 stars 11 forks source link

Proposed Internal changes #26

Closed NaruZosa closed 2 years ago

NaruZosa commented 2 years ago

Hi there!

I've just created https://github.com/NaruZosa/jncep_webui after changing my web app to import and use jncep directly instead of using subprocess.call.

There were a few hiccups along the way, but the biggest was jncep using Click, as Click is designed in such a way that it's difficult for other modules to import a function that uses Click decorators, unless you're importing from another Click CLI. Took a couple of hours to find this workaround: https://github.com/pallets/click/issues/40#issuecomment-326014129

Would you be open to accepting a pull request that replaces Click with argparse while retaining the same features and user experience? Argparse is in the stdlib, so no additional requirements, and it would also make it possible to easily make a GUI using Gooey (I'm willing to create a pull request for this as well) and also makes it possible to create unit testing with better coverage, which is difficult to do with Click.

For logging, I highly recommend changing to Loguru. It also supports coloured logging, and is very powerful and super simple to use. It is not part of stdlib, but the only dependencies for it art colorama (already present in jncep) and win32setctime, which have no dependencies of their own, and would allow dropping rich, commonmark and pygments, if I've read the jncep source correctly. I noticed that your logging levels are a bit unusual, and this seems to be because you want to have trace logging. Loguru has extra levels by default, and trace is among these. The levels available by default are trace, debug, info, success, warning, error and critical. It's also easy to add extra levels, and everything is highly customisable. The basic logger is set up with just from loguru import logger, then you can use log.trace("Message") etc. Check out my app.py for an example of the lack of boilerplate. It may look a little messy due to the InterceptHandler class, logger.remove() and the basicConfig parts, but these are only there to disconnect the jncep logger and redirect it to Loguru, normally these wouldn't be present.

Just to clarify, I'm not asking you to make these changes yourself. You've done a great job with jncep, and I'm just wondering if you'd be willing to accept a pull request for these changes before I make them :)

Also, let me know if you like the image I've used for jncep_webui and would like to use it with jncep instead. You're more than welcome to do so, and I can make a new one for jncep_webui. Alternatively, let me know what sort of image you would like for jncep and I can make one. It'd be made with Dalle, so any art style is fine (this was made with an oil painting style).

NaruZosa commented 2 years ago

Other images you may prefer :)

DALL·E 2022-09-11 17 33 54 - A book in a computer screen, kawaii anime style DALL·E 2022-09-11 17 35 00 DALL·E 2022-09-11 17 35 07

gvellut commented 2 years ago

Thank you for your feedback.

I must confess I don't want to change much to this project, as it fits my own needs the way it is right now.