geeqie icon indicating copy to clipboard operation
geeqie copied to clipboard

Duplicate search entries not being deleted with files, again

Open installgentoo opened this issue 4 years ago • 4 comments

ISSUE TYPE

  • Bug Report

GEEQIE VERSION

geeqie-1.6_p20211016
gtk3

OS / DISTRIBUTION

gentoo

SUMMARY

Back in the days of yore there was a bug https://github.com/BestImageViewer/geeqie/issues/391 It was fixed a some point, which made dedup slow as hell. Well now that i went from 1.6 to master, where dedup slowdown was fixed, guess what, the bug's back.

STEPS TO REPRODUCE

Basically add 10k images with 1000+ duplicates. Select 100 duplicates in the main window, press "Delete". Duplicates window will no longer register changes from main window. Basically dedup is broken in master.

I am using the "delete without trash folder" option.

Also notably this bug doesn't happen if you have not that many dupes in the dupes window.

installgentoo avatar Jan 16 '22 12:01 installgentoo

I have noticed a similar issue yesterday as I was removing duplicates. I used the option "Move to Trash..." and nothing had actually been removed from the folder. If I use "Delete" however, the files are deleted from the folder (permanently so that's a bit more dangerous).

I tried again today with --debug mode by copying 146 files (73 files duplicated once) and 400+ files (73 files duplicated 5 times). This time tool worked just fine and moved the files to Trash...

I am using Geeqie 1.5.1-8 as shipped on Ubuntu 20.04 LTS.

Batwam avatar Feb 24 '22 03:02 Batwam

Well no wonder, https://github.com/BestImageViewer/geeqie/commit/f08cdc035951b55335a6088f805f2068fffb8446 contains

case FILEDATA_CHANGE_DELETE:
-			while (dupe_item_remove_by_path(dw, fd->path));
+			/* Update the UI only once, after the operation finishes */

How is ui supposed to be updated when user deletes from main windows instead of dupe window? Psychicly?

installgentoo avatar May 06 '22 13:05 installgentoo

My solution because callback code is impenetrable. If dupe_item_remove is indeed the thing that's called for dupe removal and not deletion this shouldn't even be slow.

--- a/src/dupe.c	2022-04-12 03:36:50.000000000 +1200
+++ b/src/dupe.c	2022-05-07 02:24:19.136982399 +1200
@@ -60,6 +60,7 @@
 #define DUPE_DEF_HEIGHT 400
 #define DUPE_PROGRESS_PULSE_STEP 0.0001
 
+static gboolean dupe_delete_in_progress = FALSE;
 /** column assignment order (simply change them here)
  */
 enum {
@@ -514,7 +515,6 @@
 }
 */
 
-/*
 static DupeItem *dupe_item_find_path_by_list(const gchar *path, GList *work)
 {
 	while (work)
@@ -528,9 +528,7 @@
 
 	return NULL;
 }
-*/
 
-/*
 static DupeItem *dupe_item_find_path(DupeWindow *dw, const gchar *path)
 {
 	DupeItem *di;
@@ -540,7 +538,6 @@
 
 	return di;
 }
-*/
 
 /*
  * ------------------------------------------------------------------
@@ -2741,7 +2738,6 @@
 	dupe_window_update_count(dw, FALSE);
 }
 
-/*
 static gboolean dupe_item_remove_by_path(DupeWindow *dw, const gchar *path)
 {
 	DupeItem *di;
@@ -2753,7 +2749,6 @@
 
 	return TRUE;
 }
-*/
 
 static gboolean dupe_files_add_queue_cb(gpointer data)
 {
@@ -3416,6 +3411,7 @@
 static void dupe_menu_delete_cb(GtkWidget *widget, gpointer data)
 {
 	DupeWindow *dw = data;
+	dupe_delete_in_progress = TRUE;
 
 	options->file_ops.safe_delete_enable = FALSE;
 	file_util_delete_notify_done(NULL, dupe_listview_get_selection(dw, dw->listview), dw->window, delete_finished_cb, dw);
@@ -3424,6 +3420,7 @@
 static void dupe_menu_move_to_trash_cb(GtkWidget *widget, gpointer data)
 {
 	DupeWindow *dw = data;
+	dupe_delete_in_progress = TRUE;
 
 	options->file_ops.safe_delete_enable = TRUE;
 	file_util_delete_notify_done(NULL, dupe_listview_get_selection(dw, dw->listview), dw->window, delete_finished_cb, dw);
@@ -5187,7 +5184,9 @@
 		case FILEDATA_CHANGE_COPY:
 			break;
 		case FILEDATA_CHANGE_DELETE:
-			/* Update the UI only once, after the operation finishes */
+			if (!dupe_delete_in_progress) {
+				while (dupe_item_remove_by_path(dw, fd->path));
+			}
 			break;
 		case FILEDATA_CHANGE_UNSPECIFIED:
 		case FILEDATA_CHANGE_WRITE_METADATA:
@@ -5212,10 +5211,12 @@
 
 	if (!success)
 		{
+		dupe_delete_in_progress = FALSE;
 		return;
 		}
 
 	dupe_window_remove_selection(dw, dw->listview);
+	dupe_delete_in_progress = FALSE;
 }
 
 /*

@mowgli amend and commit at your discretion

installgentoo avatar May 06 '22 14:05 installgentoo

you can also add

--- a/src/dupe.c	2022-04-12 03:36:50.000000000 +1200
+++ b/src/dupe.c	2022-05-07 02:24:19.136982399 +1200
@@ -2745,7 +2745,9 @@
 	di = dupe_item_find_path(dw, path);
 	if (!di) return FALSE;
 
+	dw->color_frozen = TRUE;
 	dupe_item_remove(dw, di);
+	dw->color_frozen = FALSE;
 
 	return TRUE;
 }

to remove lag on very large sets. i doubt people care about blue-grey correctness when they delete from outside dupe window.

installgentoo avatar May 07 '22 11:05 installgentoo