-
Notifications
You must be signed in to change notification settings - Fork 71
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
Fix recursive zip extraction. #20
base: master
Are you sure you want to change the base?
Conversation
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.
awesome thanks! Just a few small thing
io::copy(&mut file, &mut output)?; | ||
let outpath = into_dir.join(file.sanitized_name()); | ||
|
||
if (&*file.name()).ends_with('/') { |
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.
can you use outpath.is_dir()
instead to be windows compatible
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 was copy pasted from zip-rs example, but yes I can.
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.
Doing that causes the program to think everything is a file. I think this is mostly due to zip-rs handling folders as files and only making the difference using the ending /.
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.
hm, that's unfortunate... Do you know if that's also the case on windows? Could you maybe setup a test zip file with nested dirs and add windows-cfg'd/linux-cfg'd tests that check this?
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 the reason for the is_dir to not work is because of the sanitized_name call, which might be removing the trailing slash.
I'm a bit(very) short on time currently, so I don't want to spend a lot of time on this bug fix writing unit tests. Sorry!
src/lib.rs
Outdated
let outpath = into_dir.join(file.sanitized_name()); | ||
|
||
if (&*file.name()).ends_with('/') { | ||
fs::create_dir_all(&outpath).unwrap(); |
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.
can you use the ?
operator instead of unwrap
s
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.
Fixed
Hello there!
Really cool crate, I like it a lot! I'll probably open more pull requests as I go.