DevHeads.net

Review Request 110922: Fix Bug 319119 - Dolphin doesn't notice when renaming failed

Review request for Dolphin, KDE Base Apps, David Faure, and Frank Reininghaus.

Description
Change the data in the model before the real renaming is done by KonqOperations::rename(),
but when the rename operation fails, revert the data changes in the model.

The problem is that DolphinView::slotRoleEditingFinished() changes the data in the model *before* the actual renaming is performed by KonqOperations/KIO.
But we need this approach for the following cases:
* Immediate feedback from the users point of view (No delay between finish renaming and DolphinView updates)
* Missing file system/dir lister signals, when there is no file system notification system (ftp, ssh, ...)

A lot of code in konq_operations.h and konq_operations.cpp is just copy and paste, to guarantee binary compatibility. (added some TODOs for KF 5.0)

This addresses bug 319119.
<a href="http://bugs.kde.org/show_bug.cgi?id=319119" title="http://bugs.kde.org/show_bug.cgi?id=319119">http://bugs.kde.org/show_bug.cgi?id=319119</a>

Diffs
dolphin/src/views/dolphinview.h 5a70c55
dolphin/src/views/dolphinview.cpp 9a4b863
lib/konq/konq_operations.h a9aec89
lib/konq/konq_operations.cpp cbb058c

Diff: <a href="http://git.reviewboard.kde.org/r/110922/diff/" title="http://git.reviewboard.kde.org/r/110922/diff/">http://git.reviewboard.kde.org/r/110922/diff/</a>

Testing
Works for me.

Tested with a remote ssh file system on a virtual machine.

Thanks,

Emmanuel Pescosta

Comments

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By Emmanuel Pescosta at 06/10/2013 - 05:22

(Updated June 10, 2013, 9:22 a.m.)

Review request for Dolphin, KDE Base Apps, David Faure, and Frank Reininghaus.

Changes
Let the old rename methods call the new renameV2 methods to avoid code duplication, but ignore the return values to guarantee binary compatibility.

Moved the RENAME flag to the end of the Operation list.

Description
Change the data in the model before the real renaming is done by KonqOperations::rename(),
but when the rename operation fails, revert the data changes in the model.

The problem is that DolphinView::slotRoleEditingFinished() changes the data in the model *before* the actual renaming is performed by KonqOperations/KIO.
But we need this approach for the following cases:
* Immediate feedback from the users point of view (No delay between finish renaming and DolphinView updates)
* Missing file system/dir lister signals, when there is no file system notification system (ftp, ssh, ...)

A lot of code in konq_operations.h and konq_operations.cpp is just copy and paste, to guarantee binary compatibility. (added some TODOs for KF 5.0)

This addresses bug 319119.
<a href="http://bugs.kde.org/show_bug.cgi?id=319119" title="http://bugs.kde.org/show_bug.cgi?id=319119">http://bugs.kde.org/show_bug.cgi?id=319119</a>

Diffs (updated)
dolphin/src/views/dolphinview.h 5a70c55
dolphin/src/views/dolphinview.cpp 9a4b863
lib/konq/konq_operations.h a9aec89
lib/konq/konq_operations.cpp cbb058c

Diff: <a href="http://git.reviewboard.kde.org/r/110922/diff/" title="http://git.reviewboard.kde.org/r/110922/diff/">http://git.reviewboard.kde.org/r/110922/diff/</a>

Testing
Works for me.

Tested with a remote ssh file system on a virtual machine.

Thanks,

Emmanuel Pescosta

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By Commit Hook at 06/10/2013 - 12:45

(Updated June 10, 2013, 4:45 p.m.)

Status
This change has been marked as submitted.

Review request for Dolphin, KDE Base Apps, David Faure, and Frank Reininghaus.

Description
Change the data in the model before the real renaming is done by KonqOperations::rename(),
but when the rename operation fails, revert the data changes in the model.

The problem is that DolphinView::slotRoleEditingFinished() changes the data in the model *before* the actual renaming is performed by KonqOperations/KIO.
But we need this approach for the following cases:
* Immediate feedback from the users point of view (No delay between finish renaming and DolphinView updates)
* Missing file system/dir lister signals, when there is no file system notification system (ftp, ssh, ...)

A lot of code in konq_operations.h and konq_operations.cpp is just copy and paste, to guarantee binary compatibility. (added some TODOs for KF 5.0)

This addresses bug 319119.
<a href="http://bugs.kde.org/show_bug.cgi?id=319119" title="http://bugs.kde.org/show_bug.cgi?id=319119">http://bugs.kde.org/show_bug.cgi?id=319119</a>

Diffs
dolphin/src/views/dolphinview.h 5a70c55
dolphin/src/views/dolphinview.cpp 9a4b863
lib/konq/konq_operations.h a9aec89
lib/konq/konq_operations.cpp cbb058c

Diff: <a href="http://git.reviewboard.kde.org/r/110922/diff/" title="http://git.reviewboard.kde.org/r/110922/diff/">http://git.reviewboard.kde.org/r/110922/diff/</a>

Testing
Works for me.

Tested with a remote ssh file system on a virtual machine.

Thanks,

Emmanuel Pescosta

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By Frank Reininghaus at 06/10/2013 - 07:02

Thanks for looking into this, Emmanuel! I'm looking forward to having one little annoyance less :-)

The Dolphin side of the patch looks good to me, and I can confirm that it works nicely.

- Frank Reininghaus

On June 10, 2013, 9:22 a.m., Emmanuel Pescosta wrote:

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By David Faure at 06/10/2013 - 06:39

Ship it!

- David Faure

On June 10, 2013, 9:22 a.m., Emmanuel Pescosta wrote:

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By David Faure at 06/10/2013 - 04:26

I don't like the duplication very much (can't the old method call the V2 method, if the only difference is the return value, which the old method would then ignore?).

Otherwise OK, as it's kind of temporary anyway. It just increases the risks of someone fixing a bug in one copy and not the other, so if we can avoid the duplication, it's better.

lib/konq/konq_operations.h
<http://git.reviewboard.kde.org/r/110922/#comment25032>

This change is BIC. Add the new flag at the end of the list.

- David Faure

On June 9, 2013, 10 p.m., Emmanuel Pescosta wrote:

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By Emmanuel Pescosta at 06/10/2013 - 12:43

Thanks for the explanation :)

- Emmanuel

On June 10, 2013, 9:22 a.m., Emmanuel Pescosta wrote:

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By David Faure at 06/10/2013 - 06:37

BIC = binary incompatible change

- David

On June 10, 2013, 9:22 a.m., Emmanuel Pescosta wrote:

Re: Review Request 110922: Fix Bug 319119 - Dolphin doesn't noti

By Emmanuel Pescosta at 06/10/2013 - 04:41

Btw.: What does BIC mean? - I only know the definition 'bank identifier code'.

- Emmanuel

On June 9, 2013, 10 p.m., Emmanuel Pescosta wrote: