Review Request 121678: Dolphin: port from KonqOperations::doDrop to the new KIO::DropJob

[David Faure]

> On Dec. 29, 2014, 8:46 p.m., Emmanuel Pescosta wrote:
> > dolphin/src/panels/folders/folderspanel.cpp, line 243
> > <https://git.reviewboard.kde.org/r/121678/diff/1/?file=335850#file335850line243>
> >
> >     Please use a lambda function which emits errorMessage on job errors. We should keep the error handling in my opinion. (same for all other error handling comments)

Ah, I see. Misunderstanding. There were two error cases. Immediate errors (QString error was set) were handled here (but this case no longer happens so I removed it).
The error in the async operation was handled by KonqOperations itself (by popping up a message box, which is against dolphin's best practices). Now that it's a proper kio job, we can either enable autoerrorhandling (and get a messagebox again) or do the error handling the dolphin way. I didn't "remove" the latter, but I can add it indeed, with a bit of guidance :)

Can you tell me if we could factorize these calls to dropUrls to have less places where to do error handling? I already removed the call in DolphinTabWidget by delegating to DolphinViewContainer, which is the one with proper error handling.
What about DolphinView::slotPasteJobResult? It's missing error handling --- but it takes care of selecting the newly created files, which we might want in all other cases too?
Is there any way to merge the dnd handling in DolphinView and DolphinViewContainer? I don't really know the dolphin design, so I'm not sure what the difference is.

For folderspanel and placespanel they should emit errorMessage, OK. Should they select the newly created files too? I guess not (different view), but better ask to be sure ;)


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121678/#review72743
-----------------------------------------------------------


On Dec. 26, 2014, 8:07 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121678/
> -----------------------------------------------------------
> 
> (Updated Dec. 26, 2014, 8:07 a.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Dolphin: port from KonqOperations::doDrop to the new KIO::DropJob
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/draganddrophelper.cpp a09faa345179fbb016b67289c550650a56639614 
>   dolphin/src/dolphintabwidget.cpp a0c9b9d8198269bcf9c560106332cdf0dd9b45a7 
>   dolphin/src/dolphinviewcontainer.h bd4141db5a2d56e6a605247eaba4da3291664645 
>   dolphin/src/dolphinviewcontainer.cpp 3954a12e6638c2abc7ab57c669e1dc1680b073a6 
>   dolphin/src/panels/folders/folderspanel.cpp f56f5a1391831666555ac31c359ac02ba794998a 
>   dolphin/src/panels/places/placespanel.cpp b04191f1c73fa42b8e9d1f18f698c2e6a4a4ee02 
>   dolphin/src/views/dolphinview.cpp cb25c6555ab2a4f3af67a762262f7868aaab687a 
>   dolphin/src/views/draganddrophelper.h c4ae974b50ab5e0af5eb9dc749d383a8bdadb2fa 
> 
> Diff: https://git.reviewboard.kde.org/r/121678/diff/
> 
> 
> Testing
> -------
> 
> Dropping between the views of a splitted dolphin mainwindow. Without modifiers, and with Ctrl, Shift, Ctrl+Shift.
> 
> Dropping onto a breadcrumb of the url navigator.
> 
> Dropping onto a tab.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20141230/f934267d/attachment.htm>