From b8881bb9468e11e9a31ddcee853426140c8d60e4 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Fri, 12 Jan 2024 00:48:44 +0300 Subject: [PATCH 01/10] batch-rename-utilities: Remove reordering function This function doesn't work when there are cycles. Remove it in favor of a another solution. --- src/nautilus-batch-rename-dialog.c | 3 - src/nautilus-batch-rename-utilities.c | 118 -------------------------- src/nautilus-batch-rename-utilities.h | 7 -- src/nautilus-file-undo-operations.c | 15 ---- 4 files changed, 143 deletions(-) diff --git a/src/nautilus-batch-rename-dialog.c b/src/nautilus-batch-rename-dialog.c index b3bc6c9cf..00678ca91 100644 --- a/src/nautilus-batch-rename-dialog.c +++ b/src/nautilus-batch-rename-dialog.c @@ -422,9 +422,6 @@ static void begin_batch_rename (NautilusBatchRenameDialog *dialog, GList *new_names) { - batch_rename_sort_lists_for_rename (&dialog->selection, &new_names, NULL, NULL, NULL, FALSE); - - /* do the actual rename here */ nautilus_file_batch_rename (dialog->selection, new_names, NULL, NULL); gtk_widget_set_cursor (GTK_WIDGET (dialog->window), NULL); diff --git a/src/nautilus-batch-rename-utilities.c b/src/nautilus-batch-rename-utilities.c index a8c312bb9..04fe4be0d 100644 --- a/src/nautilus-batch-rename-utilities.c +++ b/src/nautilus-batch-rename-utilities.c @@ -142,124 +142,6 @@ batch_rename_replace (gchar *string, return new_string; } -void -batch_rename_sort_lists_for_rename (GList **selection, - GList **new_names, - GList **old_names, - GList **new_files, - GList **old_files, - gboolean is_undo_redo) -{ - GList *new_names_list; - GList *new_names_list2; - GList *files; - GList *files2; - GList *old_names_list = NULL; - GList *new_files_list = NULL; - GList *old_files_list = NULL; - GList *old_names_list2 = NULL; - GList *new_files_list2 = NULL; - GList *old_files_list2 = NULL; - GString *new_file_name; - GString *new_name; - GString *old_name; - GFile *new_file; - GFile *old_file; - NautilusFile *file; - gboolean order_changed = TRUE; - - /* in the following case: - * file1 -> file2 - * file2 -> file3 - * file2 must be renamed first, so because of that, the list has to be reordered - */ - while (order_changed) - { - order_changed = FALSE; - - if (is_undo_redo) - { - old_names_list = *old_names; - new_files_list = *new_files; - old_files_list = *old_files; - } - - for (new_names_list = *new_names, files = *selection; - new_names_list != NULL && files != NULL; - new_names_list = new_names_list->next, files = files->next) - { - g_autoptr (NautilusFile) parent = NULL; - - new_file_name = new_names_list->data; - parent = nautilus_file_get_parent (NAUTILUS_FILE (files->data)); - - if (is_undo_redo) - { - old_names_list2 = old_names_list; - new_files_list2 = new_files_list; - old_files_list2 = old_files_list; - } - - for (files2 = files, new_names_list2 = new_names_list; - files2 != NULL && new_names_list2 != NULL; - files2 = files2->next, new_names_list2 = new_names_list2->next) - { - const char *file_name; - g_autoptr (NautilusFile) parent2 = NULL; - - file_name = nautilus_file_get_name (NAUTILUS_FILE (files2->data)); - new_name = new_names_list2->data; - - parent2 = nautilus_file_get_parent (NAUTILUS_FILE (files2->data)); - - if (files2 != files && g_strcmp0 (file_name, new_file_name->str) == 0 && - parent == parent2) - { - file = NAUTILUS_FILE (files2->data); - - *selection = g_list_remove_link (*selection, files2); - *new_names = g_list_remove_link (*new_names, new_names_list2); - - *selection = g_list_prepend (*selection, file); - *new_names = g_list_prepend (*new_names, new_name); - - if (is_undo_redo) - { - old_name = old_names_list2->data; - new_file = new_files_list2->data; - old_file = old_files_list2->data; - - *old_names = g_list_remove_link (*old_names, old_names_list2); - *new_files = g_list_remove_link (*new_files, new_files_list2); - *old_files = g_list_remove_link (*old_files, old_files_list2); - - *old_names = g_list_prepend (*old_names, old_name); - *new_files = g_list_prepend (*new_files, new_file); - *old_files = g_list_prepend (*old_files, old_file); - } - - order_changed = TRUE; - break; - } - - if (is_undo_redo) - { - old_names_list2 = old_names_list2->next; - new_files_list2 = new_files_list2->next; - old_files_list2 = old_files_list2->next; - } - } - - if (is_undo_redo) - { - old_names_list = old_names_list->next; - new_files_list = new_files_list->next; - old_files_list = old_files_list->next; - } - } - } -} - /* This function changes the background color of the replaced part of the name */ GString * batch_rename_replace_label_text (const char *label, diff --git a/src/nautilus-batch-rename-utilities.h b/src/nautilus-batch-rename-utilities.h index 39a7b12c0..bf365945c 100644 --- a/src/nautilus-batch-rename-utilities.h +++ b/src/nautilus-batch-rename-utilities.h @@ -61,10 +61,3 @@ GString* batch_rename_replace_label_text (const char *label, const gchar *substr); gchar* batch_rename_get_tag_text_representation (TagConstants tag_constants); - -void batch_rename_sort_lists_for_rename (GList **selection, - GList **new_names, - GList **old_names, - GList **new_files, - GList **old_files, - gboolean is_undo_redo); diff --git a/src/nautilus-file-undo-operations.c b/src/nautilus-file-undo-operations.c index 0a84b9dad..b18bedb1b 100644 --- a/src/nautilus-file-undo-operations.c +++ b/src/nautilus-file-undo-operations.c @@ -31,7 +31,6 @@ #include "nautilus-file.h" #include "nautilus-file-undo-manager.h" #include "nautilus-batch-rename-dialog.h" -#include "nautilus-batch-rename-utilities.h" #include "nautilus-scheme.h" #include "nautilus-tag-manager.h" @@ -1200,13 +1199,6 @@ batch_rename_redo_func (NautilusFileUndoInfo *info, files = g_list_reverse (files); - batch_rename_sort_lists_for_rename (&files, - &self->new_display_names, - &self->old_display_names, - &self->new_files, - &self->old_files, - TRUE); - nautilus_file_batch_rename (files, self->new_display_names, file_undo_info_operation_callback, self); } @@ -1233,13 +1225,6 @@ batch_rename_undo_func (NautilusFileUndoInfo *info, files = g_list_reverse (files); - batch_rename_sort_lists_for_rename (&files, - &self->old_display_names, - &self->new_display_names, - &self->old_files, - &self->new_files, - TRUE); - nautilus_file_batch_rename (files, self->old_display_names, file_undo_info_operation_callback, self); } From 9e5135057ffa46c2aea8de0f161f60ff43aa8938 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Wed, 24 Jan 2024 02:22:03 +0300 Subject: [PATCH 02/10] file: Remove unncessary condition This code was supposed to be removed in e70c6e46b77df64715b63847cd471bac49a5da9e. --- src/nautilus-file.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index 2a7c8e8fe..ea76fabf1 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -1578,8 +1578,6 @@ nautilus_file_poll_for_media (NautilusFile *file) gboolean nautilus_file_can_rename (NautilusFile *file) { - gboolean can_rename; - g_return_val_if_fail (NAUTILUS_IS_FILE (file), FALSE); /* Nonexistent files can't be renamed. */ @@ -1599,13 +1597,6 @@ nautilus_file_can_rename (NautilusFile *file) return FALSE; } - can_rename = TRUE; - - if (!can_rename) - { - return FALSE; - } - return file->details->can_rename; } From 20281558de511391ad8e43b95346d15fa8fe04bf Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Thu, 11 Jan 2024 23:15:12 +0300 Subject: [PATCH 03/10] file: Don't skip the first file from operation progress monitoring --- src/nautilus-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index ea76fabf1..efaf0d759 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2101,7 +2101,7 @@ real_batch_rename (GList *files, op->renamed_files = 0; op->skipped_files = 0; - for (l1 = files->next; l1 != NULL; l1 = l1->next) + for (l1 = files; l1 != NULL; l1 = l1->next) { file = NAUTILUS_FILE (l1->data); From 4a10fbf79182dc0cda2c8cdfdacdc9611623c747 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Wed, 24 Jan 2024 02:25:25 +0300 Subject: [PATCH 04/10] file: Don't call callback on every invalid rename This was an error in [1] in which it's calling the passed callback for every renaming failure, which was meant only for when the entire operation ends. It was copied by mistake from the code for single file rename operation. [1] be12a7510090b2ec38229b6e86bc601800d2056b --- src/nautilus-file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index efaf0d759..1077e97c7 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2119,8 +2119,7 @@ real_batch_rename (GList *files, new_file_name = nautilus_file_can_rename_file (file, new_name->str, - callback, - callback_data); + NULL, NULL); if (new_file_name == NULL) { From c7c16d5ac2c78ea8d2410a1da1903736170a556b Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Tue, 2 Jan 2024 01:25:40 +0300 Subject: [PATCH 05/10] file: Batch rename rework Move conflict resolution to nautilus-file. Use the following rough algorithm to resolve conflicts: 1- If file is the start of a chain, start processing it until the end of the chain. If it's not the start of a chain jump to step 3. The file is the begining of a chain if the targeted new name doesn't conflict with another file to be renamed, but may conflict with a name that is designated as stub. 2- Try to rename the file at the start of the chain, if can't be renamed (due to a conflict or depending on a stub), mark it as a stub. Remove the file from the chain and go to step 1 with the next file in the chain. 3- Follow the chain until: you find the beginning on the of the chain, or you reach the same file you started with. If you reached the file you started with, then it's cycle. break the cycle with a temporary name, then go to step 1 with the file at the start of the chain. Fixes https://gitlab.gnome.org/GNOME/nautilus/-/issues/1443 --- src/nautilus-file.c | 263 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 206 insertions(+), 57 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index 1077e97c7..c9299cf3f 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2082,18 +2082,22 @@ real_batch_rename (GList *files, NautilusFileOperationCallback callback, gpointer callback_data) { - GList *l1, *l2, *old_files, *new_files; + GList *l1, *l2; + g_autolist (GFile) old_files = NULL, new_files = NULL; NautilusFileOperation *op; - GFile *location; - GString *new_name; - NautilusFile *file; - GError *error; - GFile *new_file; + gsize len = 0; + g_autoptr (GHashTable) staged_names = g_hash_table_new_full (NULL, NULL, NULL, g_free); + g_autoptr (GHashTable) staged_files = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, NULL); + g_autoptr (GHashTable) stub_files = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, g_object_unref); + g_autoptr (GHashTable) unchained_files = g_hash_table_new_full (NULL, NULL, + NULL, g_object_unref); + GHashTableIter hash_iter; + GFile *key; + gchar *value; BatchRenameData *data; - - error = NULL; - old_files = NULL; - new_files = NULL; + GFile *fallback; /* Set up a batch renaming operation. */ op = nautilus_file_operation_new (files->data, callback, callback_data); @@ -2101,86 +2105,231 @@ real_batch_rename (GList *files, op->renamed_files = 0; op->skipped_files = 0; - for (l1 = files; l1 != NULL; l1 = l1->next) - { - file = NAUTILUS_FILE (l1->data); - - file->details->operations_in_progress = g_list_prepend (file->details->operations_in_progress, - op); - } - for (l1 = files, l2 = new_names; l1 != NULL && l2 != NULL; l1 = l1->next, l2 = l2->next) { - const char *new_file_name; - file = NAUTILUS_FILE (l1->data); - new_name = l2->data; + NautilusFile *file = NAUTILUS_FILE (l1->data); + GString *new_name = l2->data; + GFile *location = nautilus_file_get_location (file); + const char *new_file_name = nautilus_file_can_rename_file (file, + new_name->str, + NULL, NULL); - location = nautilus_file_get_location (file); - - new_file_name = nautilus_file_can_rename_file (file, - new_name->str, - NULL, NULL); + len++; + file->details->operations_in_progress = g_list_prepend (file->details->operations_in_progress, + op); if (new_file_name == NULL) { - op->skipped_files++; + name_is (file, new_name->str) ? op->renamed_files++ : op->skipped_files++; + + g_hash_table_insert (stub_files, g_strdup (file->details->name), location); continue; } - g_assert (G_IS_FILE (location)); + g_assert (g_hash_table_insert (staged_names, location, g_strdup (new_file_name))); + g_assert (g_hash_table_insert (staged_files, g_strdup (file->details->name), location)); + } - /* Do the renaming. */ - new_file = g_file_set_display_name (location, - new_file_name, - op->cancellable, - &error); + g_hash_table_iter_init (&hash_iter, staged_names); + g_hash_table_iter_next (&hash_iter, (gpointer *) &key, (gpointer *) &value); + while (g_hash_table_size (staged_names) > 0) + { + GFile *chain_file = key; + GFile *conflicting_staged_file = g_hash_table_lookup (staged_files, value); + gboolean is_chain_start = TRUE; - data = g_new0 (BatchRenameData, 1); - data->op = op; - data->file = file; - - g_file_query_info_async (new_file, - NAUTILUS_FILE_DEFAULT_ATTRIBUTES, - 0, - G_PRIORITY_DEFAULT, - op->cancellable, - batch_rename_get_info_callback, - data); - - if (error != NULL) + if (conflicting_staged_file != NULL) { - g_warning ("Batch rename for file \"%s\" failed", nautilus_file_get_name (file)); - g_error_free (error); - error = NULL; + g_autoptr (GFile) conflicting_staged_file_parent = g_file_get_parent (conflicting_staged_file); - op->skipped_files++; + is_chain_start = !g_file_has_parent (chain_file, conflicting_staged_file_parent); + } + + if (is_chain_start) + { + /* Process the chain leading to this file */ + gboolean skip_files = FALSE; + + while (chain_file != NULL) + { + g_autofree gchar *chain_file_old_name = g_file_get_basename (chain_file); + gchar *chain_file_new_name = g_hash_table_lookup (staged_names, chain_file); + GFile *conflicting_stub_file = g_hash_table_lookup (stub_files, + chain_file_new_name); + + if (!skip_files && conflicting_stub_file != NULL) + { + g_autoptr (GFile) conflicting_stub_file_parent = g_file_get_parent (conflicting_stub_file); + + skip_files = g_file_has_parent (chain_file, conflicting_stub_file_parent); + } + + /* Rename and skip the chain on error */ + if (!skip_files) + { + g_autoptr (GError) error = NULL; + + /* Do the renaming. */ + g_autoptr (GFile) renamed_file = g_file_set_display_name (chain_file, + chain_file_new_name, + op->cancellable, + &error); + + data = g_new0 (BatchRenameData, 1); + data->op = op; + data->file = nautilus_file_get (chain_file); + + g_file_query_info_async (renamed_file, + NAUTILUS_FILE_DEFAULT_ATTRIBUTES, + 0, + G_PRIORITY_DEFAULT, + op->cancellable, + batch_rename_get_info_callback, + data); + + if (error != NULL) + { + g_warning ("Batch rename for file \"%s\" failed: %s", + chain_file_old_name, error->message); + + /* We need to skip the rest of the files in the chain. */ + skip_files = TRUE; + } + else + { + if (g_hash_table_steal_extended (unchained_files, chain_file, + NULL, (gpointer *) &fallback)) + { + old_files = g_list_append (old_files, fallback); + } + else + { + old_files = g_list_append (old_files, g_object_ref (chain_file)); + } + + new_files = g_list_append (new_files, g_steal_pointer (&renamed_file)); + } + } + + g_hash_table_remove (staged_names, chain_file); + g_hash_table_remove (staged_files, chain_file_old_name); + + if (skip_files) + { + op->skipped_files++; + + g_hash_table_insert (stub_files, g_strdup (chain_file_old_name), + g_steal_pointer (&chain_file)); + } + + g_set_object (&chain_file, g_hash_table_lookup (staged_files, chain_file_old_name)); + } + + g_hash_table_iter_init (&hash_iter, staged_names); + g_hash_table_iter_next (&hash_iter, (gpointer *) &key, (gpointer *) &value); } else { - old_files = g_list_append (old_files, location); - new_files = g_list_append (new_files, new_file); + /* Iterate to find out if it's a chain or a cycle. */ + GFile *iter_start_file = NULL; + + while (chain_file != NULL) + { + gchar *chain_file_new_name = g_hash_table_lookup (staged_names, chain_file); + GFile *prev_chain_file = g_hash_table_lookup (staged_files, chain_file_new_name); + gboolean reached_chain_start = TRUE; + + if (prev_chain_file != NULL) + { + g_autoptr (GFile) prev_chain_file_parent = g_file_get_parent (prev_chain_file); + + reached_chain_start = !g_file_has_parent (chain_file, prev_chain_file_parent); + } + + if (reached_chain_start) + { + key = chain_file; + value = chain_file_new_name; + break; + } + + if (iter_start_file == chain_file) + { + /* We found a cycle, break it with a temporary name. */ + g_autofree gchar *random_string = g_uuid_string_random (); + gchar *prev_chain_file_old_name = chain_file_new_name; + g_autofree gchar *prev_chain_file_new_name = NULL; + GFile *renamed_file = g_file_set_display_name (prev_chain_file, + random_string, + op->cancellable, + NULL); + + g_hash_table_remove (staged_files, prev_chain_file_old_name); + g_hash_table_steal_extended (staged_names, prev_chain_file, + NULL, (gpointer *) &prev_chain_file_new_name); + + if (renamed_file != NULL) + { + g_hash_table_insert (staged_files, g_strdup (random_string), renamed_file); + g_hash_table_insert (staged_names, + renamed_file, + g_steal_pointer (&prev_chain_file_new_name)); + + g_hash_table_insert (unchained_files, renamed_file, prev_chain_file); + } + else + { + /* Unlikely to be a name conflict, stub the file. */ + op->skipped_files++; + + g_hash_table_insert (stub_files, + g_strdup (prev_chain_file_old_name), prev_chain_file); + } + + key = chain_file; + value = chain_file_new_name; + break; + } + + if (iter_start_file == NULL) + { + iter_start_file = chain_file; + } + + chain_file = prev_chain_file; + } } } + /* Attempt to rename back the files that were renamed to temporary names + * but weren't restored. Don't handle any errors though, it's likely + * futile. + */ + g_hash_table_iter_init (&hash_iter, unchained_files); + while (g_hash_table_iter_next (&hash_iter, (gpointer *) &key, (gpointer *) &fallback)) + { + g_file_move (key, fallback, G_FILE_COPY_NONE, NULL, NULL, NULL, NULL); + } + /* Tell the undo manager a batch rename is taking place if at least * a file has been renamed*/ - if (!nautilus_file_undo_manager_is_operating () && op->skipped_files != g_list_length (files)) + if (!nautilus_file_undo_manager_is_operating () && op->skipped_files != len) { op->undo_info = nautilus_file_undo_info_batch_rename_new (g_list_length (new_files)); nautilus_file_undo_info_batch_rename_set_data_pre (NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (op->undo_info), - old_files); + g_steal_pointer (&old_files)); nautilus_file_undo_info_batch_rename_set_data_post (NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (op->undo_info), - new_files); + g_steal_pointer (&new_files)); nautilus_file_undo_manager_set_action (op->undo_info); } - if (op->skipped_files == g_list_length (files)) + if (op->skipped_files == len) { - nautilus_file_operation_complete (op, NULL, error); + nautilus_file_operation_complete (op, NULL, NULL); } } From 9c1c0bc032be662680666dc536da74126b9bb202 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Sat, 13 Jan 2024 21:21:37 +0300 Subject: [PATCH 06/10] file: Don't update files in info query async callback The function used to update NautilusFile internal information after being renamed are called in an async, which causes havok since the batch renaming requires strict ordering to not add a NautiluFile to a NautilusDirectory that has the same name. Instead, collect the files that are changed while renaming and invalidate them in idle callbacks. --- src/nautilus-file.c | 110 +++++++++----------------------------------- 1 file changed, 23 insertions(+), 87 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index c9299cf3f..335170086 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2004,78 +2004,6 @@ nautilus_file_rename_handle_file_gone (NautilusFile *file, return FALSE; } -typedef struct -{ - NautilusFileOperation *op; - NautilusFile *file; -} BatchRenameData; - -static void -batch_rename_get_info_callback (GObject *source_object, - GAsyncResult *res, - gpointer callback_data) -{ - NautilusFileOperation *op; - NautilusDirectory *directory; - NautilusFile *existing_file; - char *old_uri; - char *new_uri; - const char *new_name; - GFileInfo *new_info; - GError *error; - BatchRenameData *data; - - data = callback_data; - - op = data->op; - op->file = data->file; - - error = NULL; - new_info = g_file_query_info_finish (G_FILE (source_object), res, &error); - if (new_info != NULL) - { - old_uri = nautilus_file_get_uri (op->file); - - new_name = g_file_info_get_name (new_info); - - directory = op->file->details->directory; - - /* If there was another file by the same name in this - * directory and it is not the same file that we are - * renaming, mark it gone. - */ - existing_file = nautilus_directory_find_file_by_name (directory, new_name); - if (existing_file != NULL && existing_file != op->file) - { - nautilus_file_mark_gone (existing_file); - nautilus_file_changed (existing_file); - } - - update_info_and_name (op->file, new_info); - - new_uri = nautilus_file_get_uri (op->file); - nautilus_directory_moved (old_uri, new_uri); - g_free (new_uri); - g_free (old_uri); - g_object_unref (new_info); - } - - op->renamed_files++; - - if (op->files == NULL || - op->renamed_files + op->skipped_files == g_list_length (op->files)) - { - nautilus_file_operation_complete (op, NULL, error); - } - - g_free (data); - - if (error) - { - g_error_free (error); - } -} - static void real_batch_rename (GList *files, GList *new_names, @@ -2093,11 +2021,12 @@ real_batch_rename (GList *files, g_free, g_object_unref); g_autoptr (GHashTable) unchained_files = g_hash_table_new_full (NULL, NULL, NULL, g_object_unref); + g_autoptr (GHashTable) modified_files = g_hash_table_new (NULL, NULL); GHashTableIter hash_iter; GFile *key; gchar *value; - BatchRenameData *data; GFile *fallback; + g_autoptr (GError) skip_error = NULL; /* Set up a batch renaming operation. */ op = nautilus_file_operation_new (files->data, callback, callback_data); @@ -2176,18 +2105,6 @@ real_batch_rename (GList *files, op->cancellable, &error); - data = g_new0 (BatchRenameData, 1); - data->op = op; - data->file = nautilus_file_get (chain_file); - - g_file_query_info_async (renamed_file, - NAUTILUS_FILE_DEFAULT_ATTRIBUTES, - 0, - G_PRIORITY_DEFAULT, - op->cancellable, - batch_rename_get_info_callback, - data); - if (error != NULL) { g_warning ("Batch rename for file \"%s\" failed: %s", @@ -2198,16 +2115,21 @@ real_batch_rename (GList *files, } else { + op->renamed_files++; + if (g_hash_table_steal_extended (unchained_files, chain_file, NULL, (gpointer *) &fallback)) { + g_hash_table_add (modified_files, fallback); old_files = g_list_append (old_files, fallback); } else { + g_hash_table_add (modified_files, chain_file); old_files = g_list_append (old_files, g_object_ref (chain_file)); } + g_hash_table_add (modified_files, renamed_file); new_files = g_list_append (new_files, g_steal_pointer (&renamed_file)); } } @@ -2312,6 +2234,17 @@ real_batch_rename (GList *files, g_file_move (key, fallback, G_FILE_COPY_NONE, NULL, NULL, NULL, NULL); } + g_hash_table_iter_init (&hash_iter, modified_files); + while (g_hash_table_iter_next (&hash_iter, (gpointer *) &key, NULL)) + { + NautilusFile *file = nautilus_file_get_existing (key); + + if (file != NULL) + { + nautilus_file_invalidate_attributes (file, NAUTILUS_FILE_ATTRIBUTE_INFO); + } + } + /* Tell the undo manager a batch rename is taking place if at least * a file has been renamed*/ if (!nautilus_file_undo_manager_is_operating () && op->skipped_files != len) @@ -2327,10 +2260,13 @@ real_batch_rename (GList *files, nautilus_file_undo_manager_set_action (op->undo_info); } - if (op->skipped_files == len) + if (op->skipped_files > 1) { - nautilus_file_operation_complete (op, NULL, NULL); + skip_error = g_error_new (G_IO_ERROR, G_IO_ERROR_FAILED, + "Skipped %d file(s)", op->skipped_files); } + + nautilus_file_operation_complete (op, NULL, skip_error); } void From 3cd96ff99cb694f47f6d448b1c7e7672ae605ede Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Mon, 29 Jan 2024 21:10:25 +0300 Subject: [PATCH 07/10] file-undo-operation: Fix batch rename list leak --- src/nautilus-file-undo-operations.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/nautilus-file-undo-operations.c b/src/nautilus-file-undo-operations.c index b18bedb1b..535946851 100644 --- a/src/nautilus-file-undo-operations.c +++ b/src/nautilus-file-undo-operations.c @@ -1183,7 +1183,8 @@ batch_rename_redo_func (NautilusFileUndoInfo *info, { NautilusFileUndoInfoBatchRename *self = NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (info); - GList *l, *files; + GList *l; + g_autolist (NautilusFile) files = NULL; NautilusFile *file; GFile *old_file; @@ -1209,7 +1210,8 @@ batch_rename_undo_func (NautilusFileUndoInfo *info, { NautilusFileUndoInfoBatchRename *self = NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (info); - GList *l, *files; + GList *l; + g_autolist (NautilusFile) files = NULL; NautilusFile *file; GFile *new_file; From 7b460d79aef0ae31e7fc565448a42d3b8cc94ebd Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Thu, 11 Jan 2024 16:40:17 +0300 Subject: [PATCH 08/10] test-utilities: Export hierarchy creation function --- test/automated/displayless/test-utilities.c | 2 +- test/automated/displayless/test-utilities.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/test/automated/displayless/test-utilities.c b/test/automated/displayless/test-utilities.c index f00359034..5d138d56b 100644 --- a/test/automated/displayless/test-utilities.c +++ b/test/automated/displayless/test-utilities.c @@ -57,7 +57,7 @@ empty_directory_by_prefix (GFile *parent, } } -static void +void create_hierarchy_from_template (const GStrv hier, const gchar *substitution) { diff --git a/test/automated/displayless/test-utilities.h b/test/automated/displayless/test-utilities.h index 3c1247814..4e440e2ae 100644 --- a/test/automated/displayless/test-utilities.h +++ b/test/automated/displayless/test-utilities.h @@ -16,6 +16,9 @@ void test_clear_tmp_dir (void); void empty_directory_by_prefix (GFile *parent, gchar *prefix); +void create_hierarchy_from_template (const GStrv hier, + const gchar *substitution); + void create_search_file_hierarchy (gchar *search_engine); void delete_search_file_hierarchy (gchar *search_engine); From 04ca4954297ac3cacaf53dbe518d45086c2be8ba Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Thu, 11 Jan 2024 16:41:13 +0300 Subject: [PATCH 09/10] test-utilities: Write the file name when creating a hierarchy This will be used to confirm the content after renaming operations. --- test/automated/displayless/test-utilities.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/automated/displayless/test-utilities.c b/test/automated/displayless/test-utilities.c index 5d138d56b..db20ceb42 100644 --- a/test/automated/displayless/test-utilities.c +++ b/test/automated/displayless/test-utilities.c @@ -82,6 +82,10 @@ create_hierarchy_from_template (const GStrv hier, { g_autoptr (GFileOutputStream) stream = g_file_create (file, G_FILE_CREATE_NONE, NULL, NULL); + g_autofree gchar *name = g_file_get_basename (file); + + g_output_stream_write_all (G_OUTPUT_STREAM (stream), name, strlen (name) + 1, + NULL, NULL, NULL); } } } From 8f98ecbb449a65a745377da0ec1c568f9a8f0823 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Thu, 11 Jan 2024 16:35:28 +0300 Subject: [PATCH 10/10] test/file: Add batch rename tests --- test/automated/displayless/test-file.c | 164 +++++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/test/automated/displayless/test-file.c b/test/automated/displayless/test-file.c index 1dfba7f77..e947ba9a3 100644 --- a/test/automated/displayless/test-file.c +++ b/test/automated/displayless/test-file.c @@ -4,6 +4,7 @@ #include #include #include +#include static void @@ -87,10 +88,167 @@ test_file_sort_with_self (void) g_assert_cmpint (order, ==, 0); } +typedef struct +{ + const gsize len; + GStrv expected_names; + GStrv expected_content; + gboolean *success; +} NautilusFileBatchRenameTestData; + +static void +batch_rename_callback (NautilusFile *file, + GFile *result_location, + GError *error, + gpointer callback_data) +{ + NautilusFileBatchRenameTestData *data = callback_data; + g_autoptr (GFile) root = g_file_new_for_path (test_get_tmp_dir ()); + g_autoptr (GStrvBuilder) name_builder = g_strv_builder_new (); + g_autoptr (GStrvBuilder) content_builder = g_strv_builder_new (); + g_auto (GStrv) result_names = NULL, result_content = NULL; + + g_assert_no_error (error); + g_assert_cmpint (data->len, ==, g_strv_length (data->expected_names)); + g_assert_cmpint (data->len, ==, g_strv_length (data->expected_content)); + + for (guint i = 0; i < data->len; i++) + { + g_autoptr (GFile) location = g_file_get_child (root, data->expected_names[i]); + g_autoptr (GFileInfo) info = g_file_query_info (location, NAUTILUS_FILE_DEFAULT_ATTRIBUTES, + G_FILE_QUERY_INFO_NONE, NULL, NULL); + g_autoptr (GFileInputStream) stream = g_file_read (location, NULL, NULL); + gchar content[1024]; + + g_assert_nonnull (stream); + g_assert_true (g_input_stream_read_all (G_INPUT_STREAM (stream), content, sizeof (content), + NULL, NULL, NULL)); + g_assert_true (g_input_stream_close (G_INPUT_STREAM (stream), NULL, NULL)); + + g_strv_builder_add (name_builder, g_file_info_get_display_name (info)); + g_strv_builder_add (content_builder, content); + } + + result_names = g_strv_builder_end (name_builder); + result_content = g_strv_builder_end (content_builder); + + for (guint i = 0; i < data->len; i++) + { + g_assert_cmpstr (result_names[i], ==, data->expected_names[i]); + g_assert_cmpstr (result_content[i], ==, data->expected_content[i]); + } + + *data->success = TRUE; +} + +static void +batch_rename_test (const GStrv original_names, + const GStrv expected_names) +{ + g_autoptr (GFile) root = g_file_new_for_path (test_get_tmp_dir ()); + g_autolist (NautilusFile) files = NULL; + g_autolist (GString) new_names = NULL; + const gsize len = g_strv_length (expected_names); + gboolean success = FALSE; + NautilusFileBatchRenameTestData data = { len, expected_names, original_names, &success }; + + create_hierarchy_from_template (original_names, ""); + + for (gint i = len - 1; i >= 0; i--) + { + g_autoptr (GFile) location = g_file_get_child (root, original_names[i]); + NautilusFile *file = nautilus_file_get (location); + GString *new_name = g_string_new (expected_names[i]); + + files = g_list_prepend (files, file); + new_names = g_list_prepend (new_names, new_name); + } + + nautilus_file_batch_rename (files, new_names, batch_rename_callback, &data); + + g_assert_true (success); + + /* Test undo by changing the expected names */ + data.expected_names = original_names; + success = FALSE; + + test_operation_undo (); + + batch_rename_callback (NULL, NULL, NULL, &data); + + g_assert_true (success); + + test_clear_tmp_dir (); +} + +static void +test_file_batch_rename_cycles (void) +{ + char *test_cases[][2][10] = + { + /* Small cycle */ + {{"file_1", "file_2", NULL}, + {"file_2", "file_1", NULL}}, + /* Medium cycle */ + {{"file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", "file_9", NULL}, + {"file_9", "file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", NULL}}, + /* Multi-cycle */ + {{"file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", NULL}, + {"file_8", "file_3", "file_4", "file_5", "file_6", "file_7", "file_2", "file_1", NULL}}, + }; + + g_test_bug ("https://gitlab.gnome.org/GNOME/nautilus/-/issues/1443"); + + for (guint i = 0; i < G_N_ELEMENTS (test_cases); i++) + { + batch_rename_test (test_cases[i][0], test_cases[i][1]); + } +} + +static void +test_file_batch_rename_chains (void) +{ + char *test_cases[][2][10] = + { + /* Medium chain */ + {{"file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", "file_9", NULL}, + {"file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", "file_9", "file_10", NULL}}, + }; + + for (guint i = 0; i < G_N_ELEMENTS (test_cases); i++) + { + batch_rename_test (test_cases[i][0], test_cases[i][1]); + } +} + +static void +test_file_batch_rename_replace (void) +{ + char *test_cases[][2][10] = + { + /* File Extension replacement */ + {{ + "file_1.jpg", "file_2.jpeg", "file_3.gif", "file_4.png", "file_5.webm", + "file_6.avif", "file_7.jxl", "file_8.jpeg", "file_9.bmp", NULL + }, + { + "file_1.jpg", "file_2.jpg", "file_3.gif", "file_4.png", "file_5.webm", + "file_6.avif", "file_7.jxl", "file_8.jpg", "file_9.bmp", NULL + }}, + }; + + for (guint i = 0; i < G_N_ELEMENTS (test_cases); i++) + { + batch_rename_test (test_cases[i][0], test_cases[i][1]); + } +} + int main (int argc, char *argv[]) { + g_autoptr (NautilusFileUndoManager) undo_manager = nautilus_file_undo_manager_new (); + g_test_init (&argc, &argv, NULL); g_test_set_nonfatal_assertions (); nautilus_ensure_extension_points (); @@ -107,6 +265,12 @@ main (int argc, test_file_sort_order); g_test_add_func ("/file-sort/with-self", test_file_sort_with_self); + g_test_add_func ("/file-batch-rename/cycles", + test_file_batch_rename_cycles); + g_test_add_func ("/file-batch-rename/chains", + test_file_batch_rename_chains); + g_test_add_func ("/file-batch-rename/replace", + test_file_batch_rename_replace); return g_test_run (); }