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

Missing cancel option #166

Closed
Bob590 opened this issue Nov 21, 2022 · 7 comments
Closed

Missing cancel option #166

Bob590 opened this issue Nov 21, 2022 · 7 comments

Comments

@Bob590
Copy link
Contributor

Bob590 commented Nov 21, 2022

I'm missing the cancel option... but i have found a way to add it myself.

I have to zip (STORE) a lot of files of about 130MB each – they are already compressed).
I also have the specification that this long zipping activity should be aborted at any time.
The current library implementation doesn’t allow this!

The progress callback is handled but the cancel callback is missing.

I have added the cancel callback:

int progress_cancel_callback(zip archive, void ud) {
ZipArchive
za = static_cast<ZipArchive
>(ud);
vector<ZipProgressListener*> listeners = za->getProgressListeners();
for(vector<ZipProgressListener*>::const_iterator it=listeners.begin() ; it!=listeners.end() ; ++it) {
ZipProgressListener* listener = *it;
if(listener->cancel())
return 1;
}
return 0;
}

And registered it:

int ZipArchive::close(void) {
if (isOpen()) {

    //do not handle zipSource at all because it will be deleted by libzip
    //directly when not necessary anymore

    if(!listeners.empty()) {
        zip_register_progress_callback_with_state(zipHandle, progressPrecision, progress_callback, nullptr, this);
        zip_register_cancel_callback_with_state(zipHandle, progress_cancel_callback, nullptr, this);
    }

Finally I have modified the progress listener:

class LIBZIPPP_API ZipProgressListener {
public:

    /**
     * This method is invoked while the changes are being committed during
     * the closing of the ZipArchive.
     * The value p is a double between 0 and 1, representing the overall progression.
     * The frequency of invocation of this method depends of the precision.
     *
     * Note that libzippp enforces the first invokation to be with a p-value of zero
     * and the last invokation to be with a p-value of 1. Hence, it might be possible
     * to receive multiple invokations with the same p-value, depending on the precsion
     * set in libzip.
     */
    virtual void progression(double p) = 0;
    /**
     * This method is invoked during zip/unzip operations.
     * If this function return 1 the operation is cancelled.
     * If this function return 0 the operation will continue.
     */
    virtual int cancel() = 0;
};

I have also made some small changes (basically some cast) and then added this on top of the .cpp file:

#ifdef WIN32
// Disable compiler warning for strcpy
#define _CRT_SECURE_NO_WARNINGS
#endif

All these minor changes were done to avoid VS2017 warnings when compiling.
Now the compilation is CLEAN. ;-)

If you agree I'll make a new Pull Request for your review.

@ctabin
Copy link
Owner

ctabin commented Nov 21, 2022

Hi @Bob590,

This looks great ! I always welcome any PR :-)
It would be awesome if you could separate your feature from code compilation cleaning in two separates PRs.

@Bob590
Copy link
Contributor Author

Bob590 commented Nov 21, 2022

Hi @ctabin,
Ok. I have made two separate PR requests.
The first was missing the .h update so I have made an update adding the missng .h change.
Sorry for the extra step.
:-}

@ctabin
Copy link
Owner

ctabin commented Nov 21, 2022

Hi @Bob590,

Thanks for the PR. Unfortunately both of them are failing the Travis CI. Can you have a look ?

@Bob590
Copy link
Contributor Author

Bob590 commented Nov 21, 2022

Hi @ctabin,
the first most probably fails due to the missing .h change.
How I can do/force a new CI?

@Bob590
Copy link
Contributor Author

Bob590 commented Nov 21, 2022

I see only now that also the test.cpp must be changed in order to update the ZipProgressListener...
I'm going to do it... sorry I have missed that!

@Bob590
Copy link
Contributor Author

Bob590 commented Nov 21, 2022

Done all. Now it should compile and pass the CI. :-)

@ctabin
Copy link
Owner

ctabin commented Nov 30, 2022

Done in #167.

@ctabin ctabin closed this as completed Nov 30, 2022
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

No branches or pull requests

2 participants