Fix error handling when renaming a single file
Needs RevisionPublic

Authored by alexmi on Oct 24 2019, 6:15 AM.

Details

Reviewers
elvisangelaccio
Group Reviewers
VDG
Dolphin
Summary

This removes unnecessary error handling that resulted in a couple of
bugs: One, if a rename operation failed (e.g. the new filename is too
long), the file's name did not get reverted. Two, when the new requested
filename was already in use, it would correctly ask the user for a new
one but after the operation finished successfully the file with the old
filename would be selected instead of the new one.

There's no need to manually check for a filename conflict, KIO will
handle that for us and only proceed with the operation when it should.

To be more consistent with other error messages in Dolphin, the error
message dialog when a rename operation fails was removed and instead the
errorMessage signal will be emitted. Currently this results in the error
message being displayed at the top.

Test Plan
  • Verify renaming a single file or folder still works as expected even

when the operation fails or there's a filename conflict

Diff Detail

Repository
R318 Dolphin
Branch
bugfix-rename-error-handling
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18102
Build 18120: arc lint + arc unit
alexmi created this revision.Oct 24 2019, 6:15 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptOct 24 2019, 6:15 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
alexmi requested review of this revision.Oct 24 2019, 6:15 AM

I did not add a message when the operation succeeds because that's already implemented somewhere else and is not necessary here too as far as I can tell.
And not sure if this is fine or I should have made a different diff for replacing the error dialog.

meven added a subscriber: meven.
meven added inline comments.
src/views/dolphinview.cpp
1496

You can combine this if (job->error() != 0 && job->error() != KIO::ERR_USER_CANCELED)
It is clearer IMO

alexmi updated this revision to Diff 68651.Oct 24 2019, 8:56 AM
  • Simplify a conditional statement
elvisangelaccio requested changes to this revision.Dec 28 2020, 10:03 PM

Sorry for the delay.
Patch looks good beside the inline comment. @alexmi Can you fix it?

src/views/dolphinview.cpp
1638

This is still needed, otherwise the renamed file won't get selected.

This revision now requires changes to proceed.Dec 28 2020, 10:03 PM