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

Added ability to zip a directory #488

Merged
merged 8 commits into from Jul 7, 2022
Merged

Conversation

stefins
Copy link
Contributor

@stefins stefins commented Jul 6, 2022

Related to #487

@schollz
Copy link
Owner

schollz commented Jul 6, 2022

This looks good! I think by default the receiver should unzip the directory if it receives a zipped directory using the flag when sending. Do you want to add this as well?

@stefins
Copy link
Contributor Author

stefins commented Jul 6, 2022

Should I send a flag to the receiver or unzip any file that ends with .zip ?

@schollz
Copy link
Owner

schollz commented Jul 6, 2022

A flag to the receiver. I think you already added the "TempFile" to the FileInfo so when that gets handled by the recevier they can check that.

Unzipping any zip would not be ideal because sometimes you just want to send a zip like a normal file.

@stefins
Copy link
Contributor Author

stefins commented Jul 6, 2022

@schollz Is this ok? Should we change TempFile to something else?

@schollz
Copy link
Owner

schollz commented Jul 6, 2022

This is really great!! Thanks so much.

I think just one more thing is missing - if you send a directory relative to the current directory I don't think the default behavior should be to save the zip file in the relative directory.

I.e. if you do croc --zip send ../SOMEFOLDER then it will create ../SOMEFOLDER.zip but should probably create SOMEFOLDER.zip in the current directory. Also the paths when unzipped should unzip to the current directory and maintain the folder structure of the folder.

Relative paths are really annoyingly tricky like this, thats why earlier versions of croc didn't support it. Now that it supports it I want to make sure the support continues with added features. If this is getting too complicated feel free to let me know and I can help or tag in and work on this PR too.

@stefins
Copy link
Contributor Author

stefins commented Jul 6, 2022

Thanks I will work on this further and will let you know :)

@schollz
Copy link
Owner

schollz commented Jul 6, 2022

YES ! this is so great!! I love the "Adding..." UI, very nice!

Just one more bug I found testing - ./croc --zip send ~/folder1/folder2 will attempt to send folder1, not folder2. It seems to be a slash problem because ./croc --zip send ~/folder1/folder2/ works as expected (folder2 is sent). I'm using Ubuntu and it might be OS specific (Windows has a whole slew of slash problems) so no problem if you can't reproduce, I can fix it on my end.

@stefins
Copy link
Contributor Author

stefins commented Jul 6, 2022

Yeah I was also having the same issue with the ending /.

@schollz
Copy link
Owner

schollz commented Jul 7, 2022

Awesome!!! Really amazing work, I look forward to include this in the next version

@schollz schollz merged commit 1346736 into schollz:master Jul 7, 2022
@stefins
Copy link
Contributor Author

stefins commented Jul 7, 2022

Thanks Glad I could do this! :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants