Bug 377719 - Cannot rename file with overwrite [patch]
Summary: Cannot rename file with overwrite [patch]
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: AdvancedRename-dialog (show other bugs)
Version: 5.5.0
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-17 10:55 UTC by Marek Potocki
Modified: 2022-02-01 06:23 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 6.0.0
Sentry Crash Report:


Attachments
Rename window (224.06 KB, image/png)
2017-03-17 10:55 UTC, Marek Potocki
Details
Add option to overwrite existing files in advance rename dialog (28.42 KB, patch)
2017-03-28 17:31 UTC, Simon
Details
Add option to overwrite existing files in advance rename dialog (V2) (28.46 KB, patch)
2017-03-29 12:34 UTC, Simon
Details
Add option to overwrite existing files in advance rename dialog (V3) (28.18 KB, patch)
2017-04-27 20:15 UTC, Simon
Details
Add option to overwrite existing files in advance rename dialog (V4) (27.68 KB, patch)
2017-04-29 18:13 UTC, Simon
Details
Add option to overwrite existing files in advance rename dialog (V5) (29.03 KB, patch)
2017-04-30 10:51 UTC, Simon
Details
Add option to overwrite existing files in advance rename dialog (V6) (29.11 KB, patch)
2017-05-01 21:39 UTC, Simon
Details
Add option to overwrite existing files in advance rename dialog (V7) (31.47 KB, patch)
2017-05-12 23:44 UTC, Simon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marek Potocki 2017-03-17 10:55:32 UTC
Created attachment 104608 [details]
Rename  window

Rename file with overwrite is impossible – name is highlighted in red, and OK button is inactive.
See an attachment.
Comment 1 Maik Qualmann 2017-03-17 11:23:14 UTC
An overwrite of existing files is not desired when renaming. The renaming runns automatically without question dialogs. So we check if a file with the new name already exists.

Maik
Comment 2 Marek Potocki 2017-03-17 11:39:51 UTC
(In reply to Maik Qualmann from comment #1)
> An overwrite of existing files is not desired when renaming. The renaming
> runns automatically without question dialogs. So we check if a file with the
> new name already exists.

In digiKam 4 it was possible, after confirm with dialog box. Now for rename it is necessary to delete first old file with all metadata, or use external application.
Comment 3 Simon 2017-03-28 17:31:36 UTC
Created attachment 104779 [details]
Add option to overwrite existing files in advance rename dialog

This adds a checkbox to the advanced rename dialog to allow replacing existing files by moving them to trash before renaming.

For the rename progress dialog I needed signals from iojobthreads for deletion as well. Previously there was a special case in place just for renaming. I generalized it such that for any kind of jobs signals are provided and the type of job is identified via the enum Operation that is now in its own namespace IOOperation located in iojobsthread.h. This was necessary as this enum is needed over several "levels" of classes.
Comment 4 Simon 2017-03-29 12:34:55 UTC
Created attachment 104795 [details]
Add option to overwrite existing files in advance rename dialog (V2)

Update for current master.
Comment 5 Simon 2017-04-27 12:09:12 UTC
@Gilles, Maik, Mario: Did you already have time to look at this?
Comment 6 Maik Qualmann 2017-04-27 18:24:42 UTC
I have compiler warnings here:


/home/maik/Devel/digikam-software-compilation/core/utilities/advancedrename/advancedrenameprocessdialog.cpp: In member function ‘void Digikam::AdvancedRenameProcessDialog::slotCancel(int, const QList<QUrl>&)’:
/home/maik/Devel/digikam-software-compilation/core/utilities/advancedrename/advancedrenameprocessdialog.cpp:166:50: warning: unused parameter ‘operation’ [-Wunused-parameter]
 void AdvancedRenameProcessDialog::slotCancel(int operation, const QList<QUrl>& srcFiles)

/home/maik/Devel/digikam-software-compilation/core/utilities/advancedrename/advancedrenameprocessdialog.cpp:166:80: warning: unused parameter ‘srcFiles’ [-Wunused-parameter]
 void AdvancedRenameProcessDialog::slotCancel(int operation, const QList<QUrl>& srcFiles)

/home/maik/Devel/digikam-software-compilation/core/utilities/advancedrename/advancedrenameprocessdialog.cpp: In member function ‘void Digikam::AdvancedRenameProcessDialog::slotRenameFailed(int, const QList<QUrl>&)’:
/home/maik/Devel/digikam-software-compilation/core/utilities/advancedrename/advancedrenameprocessdialog.cpp:208:56: warning: unused parameter ‘operation’ [-Wunused-parameter]
 void AdvancedRenameProcessDialog::slotRenameFailed(int operation, const QList<QUrl>& srcFiles)

/home/maik/Devel/digikam-software-compilation/core/utilities/advancedrename/advancedrenameprocessdialog.cpp:208:86: warning: unused parameter ‘srcFiles’ [-Wunused-parameter]
 void AdvancedRenameProcessDialog::slotRenameFailed(int operation, const QList<QUrl>& srcFiles)

Maik
Comment 7 Maik Qualmann 2017-04-27 18:54:33 UTC
I have now strange error messages in the console:

digikam.database: No location could be retrieved for "file:///daten/Bilder/Zugang/2017-03-08/Screenshot-004.jpg"

digikam.general: Video file  "file:///daten/Bilder/Zugang/2017-03-08/Screenshot-004.jpg"  does not exist.

digikam.general: Failed to extract video thumbnail for  "file:///daten/Bilder/Zugang/2017-03-08/Screenshot-004.jpg"

These messages I have without the patch is not. Same files renamed.
Another thing is the OK button is not activated properly when a conflict has been detected and now the checkbox is activated to move images in the trash.

Maik
Comment 8 Simon 2017-04-27 20:15:21 UTC
Created attachment 105227 [details]
Add option to overwrite existing files in advance rename dialog (V3)

Is there an option to make compilation fail on warnings? I can only find a switch to make it carry on, but it already does that.

Regarding the ok button, I cannot reproduce the problem, it activates and disables correctly here.

I will look into these weird errors (location is at least related, but video thumbnails on jpgs and raws???).
Comment 9 Simon 2017-04-27 21:45:52 UTC
(In reply to Maik Qualmann from comment #7)
> digikam.database: No location could be retrieved for
> "file:///daten/Bilder/Zugang/2017-03-08/Screenshot-004.jpg"

The offending part for this is:


    QString destFile = d->currentInfo.first.toString(QUrl::RemoveFilename) + d->currentInfo.second;
    // refresh thumbnail
    ThumbnailLoadThread::deleteThumbnail(destFile);
    // clean LoadingCache as well
    LoadingCacheInterface::fileChanged(destFile);

This was previously executed before the tests in a separate function, however it should be at the exact same position regarding real actions. Commenting these lines out makes the error disappear and has not perceivable downside. Any idea what these commands are for?

> digikam.general: Video file 
> "file:///daten/Bilder/Zugang/2017-03-08/Screenshot-004.jpg"  does not exist.
> 
> digikam.general: Failed to extract video thumbnail for 
> "file:///daten/Bilder/Zugang/2017-03-08/Screenshot-004.jpg"

I have no idea yet what this Video business is about.
Comment 10 Simon 2017-04-29 18:13:25 UTC
Created attachment 105276 [details]
Add option to overwrite existing files in advance rename dialog (V4)

Removing these two lines makes all problems (that I have) disappear. These lines were introduced in 2010 and last touched (i.e. moved around) in 2012. I don't see any reason to regenerate the thumbnail on move: The image doesn't change and e.g. copy operations don't do it either. So these lines are out of the current patch.

Maik, do you still have the problem with the ok button? And if so, can you describe how to reproduce it, because I can't - thanks in advance.
Comment 11 Maik Qualmann 2017-04-29 18:44:14 UTC
A quick test:
- compiling is OK
- Messages in the console is OK
- OK Button enabled state not.

Make sure the "move to trash" checkbox ist disabled. Open renaming and have a conflict (red entries and button is disabled). Activate the checkbox now, entries not red, but the button is disabled.

Maik
Comment 12 Simon 2017-04-29 18:49:42 UTC
Thanks for testing.
Can you provide more details or screenshots, I really can't observe the same:
http://i.imgur.com/oLK907n.png
http://i.imgur.com/QTc2muy.png
Comment 13 Maik Qualmann 2017-04-30 09:28:29 UTC
Ok, the cause is clear. Try to rename the same file with the same name. This case is not solvable, the file would be moved into the trash and can not be renamed. This case should still create a red entry or renaming should skip this file.

Maik
Comment 14 Simon 2017-04-30 10:51:17 UTC
Created attachment 105286 [details]
Add option to overwrite existing files in advance rename dialog (V5)

Ah right, I think that case wasn't handled before either, so I missed it.
Now I introduced a new "state": Unchanged. It is signified by the filenames being in gray. These are ignored, meaning the ok button is activated, unless all files are unchanged.
Comment 15 Maik Qualmann 2017-05-01 18:35:25 UTC
Simon,

I have now always red entries when I try to rename several files.

Maik
Comment 16 Simon 2017-05-01 21:39:38 UTC
Created attachment 105304 [details]
Add option to overwrite existing files in advance rename dialog (V6)

Shuffling code around usually causes side-effects, sorry for not testing.
Now I did a fair bit of renaming and did find a small issue with auto-detected sidecar files, which is also fixed in V6.
Comment 17 Simon 2017-05-12 23:44:53 UTC
Created attachment 105497 [details]
Add option to overwrite existing files in advance rename dialog (V7)

Update to master.
Comment 18 caulier.gilles 2017-05-13 09:21:56 UTC
Simon, Maik, Mario,

The next 5.6.0 is planed this Sunday. I propose to delay by 2 weeks this release to permit to generate a test AppImage including this patch (and also other recent commits from Simon), at least to see if nothing is broken through users feedback.

Gilles
Comment 19 Maik Qualmann 2017-05-13 16:15:47 UTC
Yes, this is a good idea to move the release time. I do not know if it is a good thing to move the files at the conflicts in the trash. I can easily simulate a scenario, where all files from an album move in the trash and only one remains left. Perhaps it would be better to rename the files with a unique name.

Maik
Comment 20 Mario Frank 2017-05-13 19:38:20 UTC
(In reply to caulier.gilles from comment #18)
> Simon, Maik, Mario,
> 
> The next 5.6.0 is planed this Sunday. I propose to delay by 2 weeks this
> release to permit to generate a test AppImage including this patch (and also
> other recent commits from Simon), at least to see if nothing is broken
> through users feedback.
> 
> Gilles

Hey,
I agree. I am currently quite busy with preparations for a publication. And I would like to test some scenarios and fixe some more bugs. You already pointed me to some problems.
Comment 21 Maik Qualmann 2018-03-13 21:47:21 UTC
Git commit 296632cb79d67b6166840d406182bcd71623ca6d by Maik Qualmann.
Committed on 13/03/2018 at 21:45.
Pushed by mqualmann into branch 'development/6.0.0'.

first step to factoring the IOJob class with a IOJobData container
To perform database operations only after successful file operation
or to add a dialog for user intervention
Related: bug 372763, bug 381625

M  +125  -173  libs/database/utils/dio.cpp
M  +23   -31   libs/database/utils/dio.h
M  +1    -0    libs/iojobs/CMakeLists.txt
A  +219  -0    libs/iojobs/iojobdata.cpp     [License: GPL (v2+)]
A  +91   -0    libs/iojobs/iojobdata.h     [License: GPL (v2+)]
M  +12   -8    libs/iojobs/iojobsmanager.cpp
M  +10   -16   libs/iojobs/iojobsmanager.h
M  +22   -26   libs/iojobs/iojobsthread.cpp
M  +11   -22   libs/iojobs/iojobsthread.h

https://commits.kde.org/digikam/296632cb79d67b6166840d406182bcd71623ca6d
Comment 22 Maik Qualmann 2018-03-15 20:34:23 UTC
Git commit 2b26c502d2f1cd9ef9fe661271df530922e6096a by Maik Qualmann.
Committed on 15/03/2018 at 20:31.
Pushed by mqualmann into branch 'development/6.0.0'.

now we can skip the file if the renamed file name already exists
Related: bug 372763

M  +1    -1    libs/database/utils/dio.cpp
M  +0    -1    utilities/advancedrename/advancedrenamedialog.cpp
M  +7    -4    utilities/advancedrename/advancedrenameprocessdialog.cpp

https://commits.kde.org/digikam/2b26c502d2f1cd9ef9fe661271df530922e6096a
Comment 23 Maik Qualmann 2018-03-26 18:04:37 UTC
Git commit c72fea726ea7a3051eb981a2d82757e499523940 by Maik Qualmann.
Committed on 26/03/2018 at 18:03.
Pushed by mqualmann into branch 'master'.

add possibility to rename failed items again

M  +38   -13   core/app/items/digikamimageview.cpp
M  +38   -12   core/app/views/tableview/tableview.cpp
M  +12   -3    core/utilities/advancedrename/advancedrenameprocessdialog.cpp
M  +3    -1    core/utilities/advancedrename/advancedrenameprocessdialog.h

https://commits.kde.org/digikam/c72fea726ea7a3051eb981a2d82757e499523940
Comment 24 caulier.gilles 2018-04-05 05:08:42 UTC
Simon,

Maik has work hard with iojob component with 6.0.0 code. It's the good time to review changes and to update your patch for a future integration in this release.
Comment 25 Maik Qualmann 2018-05-13 17:55:10 UTC
Git commit 83aebefea3e6dcb3c0e02060a9f9e60c11c52896 by Maik Qualmann.
Committed on 13/05/2018 at 17:53.
Pushed by mqualmann into branch 'master'.

after a failed rename, you can rename it again or rename the images by overwriting
FIXED-IN: 6.0.0

M  +2    -1    NEWS
M  +2    -2    core/app/items/imageviewutilities.cpp
M  +1    -1    core/app/items/imageviewutilities.h
M  +11   -3    core/libs/database/utils/dio.cpp
M  +1    -1    core/libs/database/utils/dio.h
M  +12   -5    core/libs/iojobs/iojob.cpp
M  +11   -1    core/libs/iojobs/iojobdata.cpp
M  +4    -1    core/libs/iojobs/iojobdata.h
M  +58   -19   core/utilities/advancedrename/advancedrenameprocessdialog.cpp

https://commits.kde.org/digikam/83aebefea3e6dcb3c0e02060a9f9e60c11c52896