D14757: Warn user before copy operation if available space is not enough

[Pino Toscano]

pino requested changes to this revision.
pino added a comment.
This revision now requires changes to proceed.


  Hint: please make sure you build **and** test the next versions of your patches, otherwise it is just a waste of everybody's time.

INLINE COMMENTS

> copyjob.cpp:891
> +        if (m_totalSize > m_freeSpace) {
> +            SimpleJobPrivate *sjp;
> +            int msgRes;

Uninitialized pointer, this will crash two lines later...
Also, this is the base class of the private class used for this job, and this function is part of that class already; so why aren't you just invoking it?

> copyjob.cpp:893
> +            int msgRes;
> +            msgRes = sjp->requestMessageBox(JobUiDelegateExtension::WarningYesNo,
> +                                            QString("Warning!"),

The order of the parameters does not match at all the actual parameters of the function.
Also, all the text/caption strings **must** be translated.

> copyjob.cpp:899
> +                                                 "Do you still want to continue?",
> +                                                  m_dest.path(),
> +                                                  KIO::convertSize(m_totalSize),

Assuming `m_dest` is a local file, then `toLocalFile()` is the right function to call (`path()` will give a different result on Windows).

> copyjob.cpp:909
> +
> +            if (msgRes == KMessageBox::Yes) {
> +                goto yes;

The return value is `JobUiDelegateExtension::MessageBoxType`, not `KMessageBox::ButtonCode`.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D14757

To: shubham, pino, dfaure
Cc: dfaure, pino, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180812/2e318aa6/attachment-0001.html>