-
Couldn't load subscription status.
- Fork 176
Download books and extras into separate directories #27
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
Conversation
If path.separate is set to true in the configuration file, downloads books and extras into separate directory. Examples: ebooks/TITLE - AUTHOR/TITLE.pdf ... ebooks/TITLE - AUTHOR/extras/TITLE.png Note that if you use separate directories path.extras behaves differently. The code is only a suggestion (preview) of new functionality, but works properly with local download. Other features to check (possible complications with Google Drive upload). Instead of setting this in the configuration file, separate or not directory can be changed by additional option from the command line.
Hi! I like the idea, I will add few hints in the code directly, let me know what do you think!
config/dev.cfg
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to avoid any regression, I suggest to
# leave the old path: unfortunately I have written no tests... my fault
path.extras=ebooks/extras
# add something like the following, default must be FALSE
path.separate=false
path.separate.ebooks=MY/EBOOKS/FOLDER
path.separate.extras=MY/EXTRAS/FOLDER
Updated Maybe instead of having path.separate=false just check if path.separate.extras is not empty ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I also thought if true/false is actually needed. This can be changed. path.separate.ebooks and path.separate.extras is also a good idea. I will try to change this in the near future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first package of improvements: 5015a87
Config now do not even have to contain path.ebooks and path.extras. There can be only path.separate.ebooks and path.separate.extras.
script/packtpub.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- use regexp on
titleandauthorto remove any extraneous character: restrict to [a-zA-Z] for example - try to avoid any space, I suggest to use
_as separator - maybe verify that if the folder is nested for some reason, the script doesn't throw any error (equivalent of unix
mkdir -p /my/path/to/new/folder - I'm not a python ninja, so don't know if
XXX == "true"is the best way for comparison or you prefer to check simply ifpath.separate.extrasis not an empty string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're definitely right - checking variables is always desirable. Even though here should not appear anything wrong. I will try to add a regex for this. Probably [a-zA-Z] with single space, hyphen and comma.
Regarding to _ instead of spaces: I'm not entirely convinced. This is great for file names but for directory names less.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On unix system usually is desiderable to avoid empty space, if you prefer you can have also
path.separate.space=_
as default, so is configurable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moreover to concatenate the string you could use .format() or any equivalent to make it more readable
Ok, I will try to do this in my free time, but I can not promise when exactly.
No problem. Let me know if you have any other idea or suggestion.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, I like it in this way with the has_option
script/packtpub.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll wait to merge the request when you will add the sanity (regexp) check and the path.separate.space.
Maybe something like this should be already enough
.encode('ascii', 'ignore').replace(' ', '_')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did it 😄
path.separate.space=? is for space.
Thanks for your contribution!
Uh oh!
There was an error while loading. Please reload this page.
Short description
If
path.separateis set totruein the configuration file, downloads books and extras into separate directories.Examples
If
path.separateis set totrue,path.ebooks=ebooksandpath.extras=extras, ebooks and extras are downloaded to the following locations:ebooks/TITLE - AUTHOR/TITLE.pdfebooks/TITLE - AUTHOR/extras/TITLE.pngNote that if you use separate directories,
path.extrasbehaves differently:If
path.separateis set tofalse,path.ebooks=ebooksandpath.extras=ebooks/extras, ebooks and extras are downloaded to the following locations:ebooks/TITLE.pdfebooks/extras/TITLE.pngDisclaimer
The code is only a suggestion (preview) of new functionality, but works properly with local downloads. Other features to check (possible complications with Google Drive upload). Instead of setting this in the configuration file, separate or not directory can be changed by additional option from the command line.