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
Add DownloadToEpub.koplugin #9943
base: master
Are you sure you want to change the base?
Conversation
Using another working link: Guess I need that other PR of yours :) but enough playing for today :) |
Ok, a bit more playing. |
He's playing with it! How nice (: You'll want the latest commits (incoming...), which should fix the errors you'd been having. |
41678cc
to
273f869
Compare
Is there anything I need to change for this PR to be integrated? Also, well, do we want this PR integrated? |
I have zero interest in the plugin from a user POV, and my past experience with reviewing a from-scratch massively modular PR was... not a fun one (and made actually grasping how it all fit together basically impossible without resorting to a local IDE), so I have yet to find the block of time + will necessary to review it ;). |
Ahhhh I am haunted by the same issues my other work! Alas :'( I'm starting to see what you mean by the modularity being undesirable. There definitely can be some ways to reduce the file count by 50 percent at least. (On a side note: no longer taking Java so it can't get any worse, right?) Anyways ... it sounds like I should trash this and the other thing because they're so far gone and ... tbh .... I'm not using my ereader much more anyways :') |
I'm in the same boat btw. I'd certainly like to take a better look, but the code structure is less inviting to me. I can't grok it very well just looking at the GH diff or Reviewable due to the unnecessarily complex inheritance. If you compare the recent vocabbuilder plugin for example that was very clear for me to read. I'm not really sure how to turn that into useful/actionable feedback, which is why I hadn't responded yet. But for example, I don't really understand why "request" and "requestfactory" are separate files. I know the factory pattern is a popular thing in Java but my understanding of it is a bit different (but possibly wrong or outdated). I'd envision the pattern more like you have the request class and then you have several different factories depending on what type of request you're doing. I'm not sure how much sense that makes here regardless but we don't even have a postrequestfactory vs a getrequestfactory or something. :-) A similar example is the htmldocument stuff. To me there's no clear reason to split it up into 7 (?) files. I think I'd basically just have one or two files, the htmldocument and the template. That's mainly because at this point those are all one or two functions in an entirely new file/object. (Also see the ResourceIterator in webpageadapter.) In short, it comes across to me as using a pattern for the sake of using a pattern (or perhaps: to learn the pattern?) rather than because it's necessarily useful here. Anyway, as in writing good text, writing good code also often comes down to rewriting. That's not trashing it. ;-) |
Yeah, I was building it with a mind towards the future, where there would need to be many different factories for other subscription services. Probably that's why it's so bloated--those other services never appeared.
Actually, ha, yeah, the epub builder stuff twined with me learning about these types of patterns and then going whole hog on them. Oooopsies :)
True true true...! That would make it 10x easier to work with, too, since I'm not switching buffers so much to make a little change. Hmmm! |
Plugin adds a "Download To EPUB" button to external links.
This change is