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 with your patch.
  If works for me without, and I bet it fails with your patch, because "if" 
isn't an executable.
  (https://commits.kde.org/kio/0e28930e6748fb45d152e7c3023a034b7db23854 shows 
what executablePath() will return for that Exec line).
  
  The reason this code was in slotProcessExited is that at that point we know 
something went wrong, and we can try to find out what happened (and if we get 
the diagnosis slightly wrong, the harm is minimal, an error did happen in any 
case). OTOH your patch will fail with any sort of shell command in the Exec 
line, that doesn't start with an executable name.
  
  I think the actual fix is to call a virtual method for that messagebox, so 
that it can be implemented differently in apps that don't like message boxes 
(not just plasma but also e.g. dolphin).

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

Reply via email to