hein updated this revision to Diff 27046.
hein added a comment.
New approach.
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10405?vs=26821=27046
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D10405
AFFECTED FILES
src/widgets/krun.cpp
To:
hein added a comment.
It's totally conceivable for e.g. a KPart or a Plasma plugin to open a
QWidget-based window where the KMessageBox is appropriate, and if there's only
a global interface instance and Plasma overrides it to hide all message boxes
it's going to break KIO users in plugins.
dfaure added a comment.
Which plugins?
This stuff is for the application to decide how it wants to handle user
interactions like the rename dialog, the skip dialog, the confirm-deletion
dialog, and messageboxes.
KIOWidgets provides a default implementation (with modal dialogs), but it
hein added a comment.
There's a Job::setUiDelegateExtension though in addition to
KIO::setDefaultJobUiDelegateExtension. A global would wreak havoc with stuff
like plugins, no?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10405
To: hein, dfaure, davidedmundson,
dfaure added a comment.
What? no no, KIO::defaultJobUiDelegateExtension()->requestMessageBox() is a
kind of singleton, no refactoring needed.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10405
To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh
hein added a comment.
Thanks for the solution for sure, but - it requires writing a million lines
of boilerplate, extensively refactoring KRun and adding reams of new overloads
to its API (there would need to be some way to pass an instance of that
interface to all these - not internal -
dfaure added a comment.
Come on, this is fun! Well, unless you feel like I took the fun out of it by
providing a solution...
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10405
To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh
hein added a comment.
/o\ I don't like life anymore
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10405
To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh
dfaure added a comment.
The statics can be refactored, if they're internal ;)
What's more troublesome is the KRun instance might have gone away by the time
KProcessRunner ::slotProcessExited is called, though (which is why it's done in
a different class).
So maybe what's needed is
hein added a comment.
@dfaure I don't see how the virtual thing is going to work - there's numerous
static affected codepaths so there's no 'this' to pass down to call the
non-static error handler on.
I could add a new "silent" RunFlag and pass that around to quiet the message
box
dfaure added a comment.
Yes. Well, you can't add new virtual methods, but these two already exist:
virtual void handleInitError(int kioErrorCode, const QString );
virtual void handleError(KJob *job);
I think you should be able to (ab)use the first one for this purpose.
hein added a comment.
Yeah, I had a feeling there were other design considerations here, but
they're not documented/commented in the code so the only way to find out was to
show you a patch :)
You want this virtual method where? In KRun? And then Plasma should subclass
KRun?
dfaure added a comment.
(FWIW I tested my desktop file in dolphin, not in plasma)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10405
To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
If only it was that simple, I would have done this years ago.
Download http://www.davidfaure.fr/2018/weirdcommand.desktop and then running
that desktop file, without and then
mart added a comment.
+1 from me
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10405
To: hein, dfaure, davidedmundson, mart
Cc: #frameworks, michaelh, ngraham
hein updated this revision to Diff 26821.
hein added a comment.
Remove unrelated changes.
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10405?vs=26820=26821
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D10405
AFFECTED FILES
hein added a comment.
Argh, sorry for the noisy diff, I didn't do commit -a but arc did I guess ...
I'll clean this up, one sec.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10405
To: hein, dfaure, davidedmundson, mart
Cc: #frameworks, michaelh, ngraham
hein created this revision.
hein added reviewers: dfaure, davidedmundson, mart.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
hein requested review of this revision.
REVISION SUMMARY
Without this patch, runCommandInternal tries
18 matches
Mail list logo
