-
Notifications
You must be signed in to change notification settings - Fork 43
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
Feature/use counter for names #135
Feature/use counter for names #135
Conversation
Update to the new Version
Latest update
…ace names with counter. Starting from 0
Also, change the address for #131 |
@@ -13,6 +13,8 @@ public IOutputer CreateOutput(OutputFormat format) | |||
return new FolderOutput(); | |||
case OutputFormat.CBZ: | |||
return new CbzOutput(); | |||
case OutputFormat.Counter: // no IOutputer implementation |
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.
This design can be improved. For example, we should make it an option in FolderOuput or and Post-Output.
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.
Not sure what you mean by this.
The main point was to set the simple meaningful name instead of long ones before saving it on the FileSystem.
I set it as "OutputFormat" because it's the easiest way to access it without changing "DownloadChapterTask".
Could you please explain your suggestion a little bit more.
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 think output format is folder or cbz but naming files should be a difference mechanism.
Currently, we download images into a temp folder and create 'output' folder or cbz based on images in temp folder. We can add a step to renaming files in the temp folder before create output. It's clear and less complex.
Later, we can extend it to more complex renaming scenario or provide many setting to rename files much easier.
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.
Currently, images are saved in the temp folder with their original names ("DownloadImages" method). And according to #133 it is the place where it failed. To prevent it from happening, I have put the renaming operation there.
You proposal require making the changes in "DownloadChapterTask" class and in the "DownloadImages" method. And if it's pretty clear what to do with "DownloadChapterTask" class, "DownloadImages" method is not so clear.
Should we simply check and cut the name if it has a long name, or should we save the files with random names and return a new class which will keep the temp names with original names (and maybe counter) and later uses them with "Output" methods?
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.
@NguyenDanPhuong ping
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 don't think this is relevant now. I will close this. Thanks.
For those who don't like file names as "Ntkgp-", "fC9nPR0AhT" e.t.c. and like simple numbers.
Also, a possible solution for #133 and #78