Skip to content
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 Request]: BitOutputArchive::addFiles( const tstring& in_dir... should take options.only_files from an argument instead of 'false' #226

Closed
1 task done
dhananjay-gune opened this issue Jul 30, 2024 · 6 comments
Assignees

Comments

@dhananjay-gune
Copy link

dhananjay-gune commented Jul 30, 2024

Feature description

I observe that the 7z file created by using BitOutputArchive::addFiles( const tstring& in_dir... api, only captures the files under the in_dir. It does not capture the sub-dirs in the in_dir for e.g. if the in_dir were to have the below structure:
in_dir\FileA.txt
in_dir\subDir
in_dir\subDir\FileB.txt

when I call BitArchiveReader.items(), it reports only 2 items - FileA.txt and FileB.txt it does not report the subDir.

This is because the options.files_only is hardcoded to true. When changed via debugger to false, the reader reports the subDir as well.

image

Would it be possible to take it from an argument?

I tried setting mArchiveCreator.retainDirectories() but it did not have any effect.

Env: Win10.

Additional context

No response

Code of Conduct

@dhananjay-gune dhananjay-gune changed the title [Feature Request]: BitOutputArchive::addFiles( const tstring& in_dir... should take options.only_files from archiveCreator instead of 'false' [Feature Request]: BitOutputArchive::addFiles( const tstring& in_dir... should take options.only_files from an argument instead of 'false' Jul 30, 2024
@rikyoz
Copy link
Owner

rikyoz commented Jul 30, 2024

Hi!

Would it be possible to take it from an argument?

The problem I have with this change is that it would make the name of the method addFiles a bit ambiguous. By design, the method only adds files, not folders, hence the hardcoded value of only_files. If the method took an argument for this option, it would not just add files, contrary to what its name implies.

But I think the library should make it possible to do something like what you want, no doubt. So I think the best way would be to add a method like addDirectoryContent (or just an overload of addItems) which adds everything it finds in a directory.

There is already addDirectory, but it keeps the indexed directory as the root of the files added to the archive, so I think adding a new method is the best way to achieve this.

when I call BitArchiveReader.items(), it reports only 2 items - FileA.txt and FileB.txt it does not report the subDir.

Just to be sure: after an addFiles call, the output archive should contain two items: FileA.txt and subDir\FileB.txt (not just FileB.txt), i.e., the metadata of the subDir folder is not stored in the archive, but the FileB.txt should have the subDir\ in its path. At least, this is the expected behavior.

Anyway, I'll add the new method to the library, possibly in the upcoming v4.0.8 of the library.

Thanks for pointing this out!

@dhananjay-gune
Copy link
Author

Thank you very much for a prompt reply, explanation, and the solution.
I'll integrate the new version when ready.

I just wanted to let you know that your library is very good and the work you are doing is really making a difference in the world.

rikyoz added a commit that referenced this issue Jul 31, 2024
@rikyoz
Copy link
Owner

rikyoz commented Jul 31, 2024

Thank you very much for a prompt reply, explanation, and the solution.

You're welcome!

I just pushed a commit to the hotfix/v4.0.8 branch that adds the addDirectoryContents method to BitOutputArchive.

I just wanted to let you know that your library is very good and the work you are doing is really making a difference in the world.

Thank you for your kind words and support! I'm glad to hear that this library is making a positive impact.
Feel free to reach out if you have any further questions or issues.

@dhananjay-gune
Copy link
Author

Thanks! I'll take the 4.0.8 after you release it.

@dhananjay-gune
Copy link
Author

Shouldn't the same functionality be extended to the classes that wrap the BitOutputArchive? i.e. those who implement BitAbstractArchiveCreator e.g. BitFileCompressor, BitArchiveWriter, ... , Or perhaps in the BitAbstractArchiveCreator class itself... I don't know....

In other words, if I only wanted to use the BitOutputArchive.addDirectoryContents() or some other methods of the class, which archive creator should I inject?

@rikyoz
Copy link
Owner

rikyoz commented Aug 10, 2024

Shouldn't the same functionality be extended to the classes that wrap the BitOutputArchive? i.e. those who implement BitAbstractArchiveCreator e.g. BitFileCompressor, BitArchiveWriter, ... , Or perhaps in the BitAbstractArchiveCreator class itself... I don't know....

This is automatically done for BitArchiveWriter, as it extends BitOutputArchive (and BitAbstractArchiveCreator), and for BitArchiveEditor, which extends BitArchiveWriter.

BitFileCompressor is different though, as it internally uses BitOutputArchive, instead of inheriting from it. I think it makes sense to add a function like compressDirectoryContents(), given it already provides a compressDirectory() method. I'll add it asap to the v4.0.8 branch.

In other words, if I only wanted to use the BitOutputArchive.addDirectoryContents() or some other methods of the class, which archive creator should I inject?

BitFileCompressor is meant to be used when you want to create multiple archives using the same settings and callbacks. In all other cases, e.g., creating only one archive, or creating different archives with different settings, BitArchiveWriter is usually a better choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants