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

shubham updated this revision to Diff 41655. shubham edited reviewers, added: broulik; removed: cfeck. shubham added a comment. Done above requested changes. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14757?vs=39531&id=41655 REVISION DETAIL https://phabr

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

shubham added inline comments. INLINE COMMENTS > pino wrote in copyjob.cpp:891 > 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?

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

shubham added a comment. @dfaure @pino help? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14757 To: shubham, pino, dfaure, cfeck Cc: dfaure, pino, kde-frameworks-devel, michaelh, ngraham, bruns

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

shubham added inline comments. INLINE COMMENTS > pino wrote in copyjob.cpp:909 > The return value is `JobUiDelegateExtension::MessageBoxType`, not > `KMessageBox::ButtonCode`. There is no enum for Button Code inside JobUiDelegatextension. So how to take that into consideration? REPOSITORY R

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

shubham added a reviewer: cfeck. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14757 To: shubham, pino, dfaure, cfeck Cc: dfaure, pino, kde-frameworks-devel, michaelh, ngraham, bruns

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

shubham added a comment. A question requestMessageBox( ) is a "pure virtual" function inside "jobUiDelegateExtension" class which is implemented in "jobUiDelegate" class which is the derived class of "jobUiDelegateExtension". When I resolve requestMessageBox() using : : like this (jobUiD

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

dfaure requested changes to this revision. dfaure added inline comments. INLINE COMMENTS > CMakeLists.txt:153 > KF5::DBusAddons# KDEInitInterface > + KF5::WidgetsAddons > ) Please remove > copyjob.cpp:49 > > +#include > + Please remove > copyjob.cpp:892 > +Simple

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

dfaure added a comment. You can just emit the warning signal, from the job. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14757 To: shubham, broulik, ngraham, pino Cc: dfaure, pino, kde-frameworks-devel, michaelh, ngraham, bruns

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

shubham edited reviewers, added: dfaure; removed: broulik, ngraham. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14757 To: shubham, pino, dfaure, broulik Cc: dfaure, pino, kde-frameworks-devel, michaelh, ngraham, bruns

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

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_

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

shubham updated this revision to Diff 39531. shubham added a comment. use simpleJobPrivate REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14757?vs=39492&id=39531 REVISION DETAIL https://phabricator.kde.org/D14757 AFFECTED FILES src/core/CMakeLists.txt s

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

shubham added a comment. @dfaure is it that message boxes cant be used in non gui library as suggested by pino. Is there any harm in using kmessagebox instead? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14757 To: shubham, broulik, ngraham, pino Cc: dfaure, pino, kd

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

shubham added a comment. In D14757#306938 , @pino wrote: > Also, considering this is in a job in `KIOCore` (i.e. non-gui library), I suspect that using a message box directly is the wrong way to do it. @dfaure what is your opinion on that? REPO

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

dfaure added a comment. Yes, Pino is right. Trust Pino ;) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14757 To: shubham, broulik, ngraham, pino Cc: dfaure, pino, kde-frameworks-devel, michaelh, ngraham, bruns

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

dfaure added a comment. Ah, hmm, my suggestion would allow to display a warning messagebox, but not to offer "continue/cancel" type of interaction. You can use SimpleJobPrivate::requestMessageBox instead, with the type argument set to JobUiDelegateExtension::WarningContinueCancel, instea

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

shubham added a comment. In D14757#306938 , @pino wrote: > Also, considering this is in a job in `KIOCore` (i.e. non-gui library), I suspect that using a message box directly is the wrong way to do it. Most probably you need to use the UI delega

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

pino requested changes to this revision. pino added a comment. This revision now requires changes to proceed. Also, considering this is in a job in `KIOCore` (i.e. non-gui library), I suspect that using a message box directly is the wrong way to do it. Most probably you need to use the UI del

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

shubham created this revision. shubham added reviewers: broulik, ngraham. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. shubham requested review of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde