After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770586 - General batch rename improvements
General batch rename improvements
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-30 08:34 UTC by Carlos Soriano
Modified: 2016-09-22 09:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
batch-rename-utilities: 0 pad the track number (988 bytes, patch)
2016-08-30 08:34 UTC, Carlos Soriano
committed Details | Review
batch-rename-dialog.ui: make rows unselectable (1.94 KB, patch)
2016-08-30 08:34 UTC, Carlos Soriano
committed Details | Review
batch-rename-dialog: use constants (1.92 KB, patch)
2016-08-30 08:35 UTC, Carlos Soriano
committed Details | Review
dialog-batch-rename.c: increase row margin (1.02 KB, patch)
2016-08-30 08:35 UTC, Carlos Soriano
none Details | Review
batch-rename-dialog.c: increase row margin (1.02 KB, patch)
2016-08-30 08:35 UTC, Carlos Soriano
committed Details | Review
batch-rename-utilities: print warnings if query goes wrong (1.19 KB, patch)
2016-08-31 12:35 UTC, Carlos Soriano
committed Details | Review
batch-rename-utilities: protect against null error (1.62 KB, patch)
2016-08-31 13:15 UTC, Carlos Soriano
committed Details | Review
batch-rename-utilities: fix query limit error (2.03 KB, patch)
2016-09-05 21:12 UTC, Alexandru Pandelea
committed Details | Review
batch-rename-dialog: use GTask appropiately (13.64 KB, patch)
2016-09-07 14:10 UTC, Carlos Soriano
committed Details | Review
batch-rename-dialog: merge the general conflict check handling (9.34 KB, patch)
2016-09-07 14:10 UTC, Carlos Soriano
committed Details | Review
batch-rename-dialog: use array-for instead of if-elses (7.84 KB, patch)
2016-09-12 09:37 UTC, Carlos Soriano
committed Details | Review
batch-rename-dialog: use 4 spaces for identation (1.57 KB, patch)
2016-09-12 09:37 UTC, Carlos Soriano
committed Details | Review
batch-rename-dialog: major refactoring (104.99 KB, patch)
2016-09-12 09:37 UTC, Carlos Soriano
none Details | Review
batch-rename-dialog: major refactoring (106.42 KB, patch)
2016-09-12 11:52 UTC, Carlos Soriano
committed Details | Review
nautilus-file: copy parameter data in batch renaming (1.45 KB, patch)
2016-09-21 18:30 UTC, Carlos Soriano
committed Details | Review
nautilus-file: plug leak (1.02 KB, patch)
2016-09-21 18:30 UTC, Carlos Soriano
committed Details | Review
batch-rename-dialog: Monitor directories for changes (6.71 KB, patch)
2016-09-21 21:06 UTC, Alexandru Pandelea
none Details | Review
batch-rename-dialog: remove unused vars (936 bytes, patch)
2016-09-22 09:31 UTC, Carlos Soriano
committed Details | Review
batch-rename-utilities: fix variable type (851 bytes, patch)
2016-09-22 09:31 UTC, Carlos Soriano
committed Details | Review
batch-rename-utilities: remove unused function (1.12 KB, patch)
2016-09-22 09:31 UTC, Carlos Soriano
committed Details | Review

Description Carlos Soriano 2016-08-30 08:34:48 UTC
Bug tracker for general code improvements for batch renaming
Comment 1 Carlos Soriano 2016-08-30 08:34:51 UTC
Created attachment 334413 [details] [review]
batch-rename-utilities: 0 pad the track number

Usually the track numbers are 0 padded by 2. Do that in batch renaming
for familiarity.
Comment 2 Carlos Soriano 2016-08-30 08:34:57 UTC
Created attachment 334414 [details] [review]
batch-rename-dialog.ui: make rows unselectable

There is no point on select them, and it overrides background color
when selecting conflicts.
Comment 3 Carlos Soriano 2016-08-30 08:35:02 UTC
Created attachment 334415 [details] [review]
batch-rename-dialog: use constants
Comment 4 Carlos Soriano 2016-08-30 08:35:07 UTC
Created attachment 334416 [details] [review]
dialog-batch-rename.c: increase row margin
Comment 5 Carlos Soriano 2016-08-30 08:35:38 UTC
Created attachment 334417 [details] [review]
batch-rename-dialog.c: increase row margin
Comment 6 Carlos Soriano 2016-08-30 08:37:06 UTC
Attachment 334413 [details] pushed as cf7c8ac - batch-rename-utilities: 0 pad the track number
Attachment 334414 [details] pushed as 122668b - batch-rename-dialog.ui: make rows unselectable
Attachment 334415 [details] pushed as c015166 - batch-rename-dialog: use constants
Attachment 334417 [details] pushed as bbf896e - batch-rename-dialog.c: increase row margin
Comment 7 Carlos Soriano 2016-08-31 12:35:06 UTC
Created attachment 334528 [details] [review]
batch-rename-utilities: print warnings if query goes wrong
Comment 8 Carlos Soriano 2016-08-31 13:00:27 UTC
Comment on attachment 334528 [details] [review]
batch-rename-utilities: print warnings if query goes wrong

Attachment 334528 [details] pushed as 4fcca2e - batch-rename-utilities: print warnings if query goes wrong
Comment 9 Carlos Soriano 2016-08-31 13:15:40 UTC
Created attachment 334532 [details] [review]
batch-rename-utilities: protect against null error
Comment 10 Carlos Soriano 2016-08-31 13:16:49 UTC
Comment on attachment 334532 [details] [review]
batch-rename-utilities: protect against null error

Attachment 334532 [details] pushed as cfa0d2c - batch-rename-utilities: protect against null error
Comment 11 Alexandru Pandelea 2016-09-05 21:12:06 UTC
Created attachment 334855 [details] [review]
batch-rename-utilities: fix query limit error

If there were too many files in batch renaming, the tracker query would
give the following error: "parser stack overflow", because the limit for
the expression tree depth was hit for the file name filter.

The fix was to use a filter that doesn't expand that much in sql.
Comment 12 Carlos Soriano 2016-09-07 14:09:49 UTC
Review of attachment 334855 [details] [review]:

Cool! Good to know a solution for this. Did Carlos Garnacho reviewed this? If so, it looks good to me afaics. Thanks!
Comment 13 Carlos Soriano 2016-09-07 14:10:07 UTC
Created attachment 334988 [details] [review]
batch-rename-dialog: use GTask appropiately

The Gtask has to finish only when all the internal async call are ready,
doing the other way around is conceptually wrong and will cause problems
if we rely on the task for checking the status of the general operation.

For that implement a correct GTask handling, waitig with a mutex and
condition for all async calls to be ready, and only then mark the task
as finished.
Comment 14 Carlos Soriano 2016-09-07 14:10:13 UTC
Created attachment 334989 [details] [review]
batch-rename-dialog: merge the general conflict check handling

We were checking separately when a directory had different parents
than just one.
However having just one parent is a special case of having multiple
parents.
With the latest refactoring, we can merge the code now.
Comment 15 Carlos Soriano 2016-09-07 14:11:03 UTC
Attachment 334988 [details] pushed as 8195468 - batch-rename-dialog: use GTask appropiately
Attachment 334989 [details] pushed as a534f94 - batch-rename-dialog: merge the general conflict check handling
Comment 16 Carlos Soriano 2016-09-12 09:37:33 UTC
Created attachment 335345 [details] [review]
batch-rename-dialog: use array-for instead of if-elses

We were doing the same for several items. Use and array and a for
to iterate on them.
Comment 17 Carlos Soriano 2016-09-12 09:37:40 UTC
Created attachment 335346 [details] [review]
batch-rename-dialog: use 4 spaces for identation
Comment 18 Carlos Soriano 2016-09-12 09:37:46 UTC
Created attachment 335347 [details] [review]
batch-rename-dialog: major refactoring

This is a refactoring that is difficult to split in several pieces.

This patch implements the following changes:
- Use loops instead of serie of if/elses.
- Use enums and static arrays to define a static serie of tag types
and properties.
- Use enums for ordering and retrieval of properties of tags instead of
searching.
- Merge common code.
- Extract common code in functions.
- Mark for translation some strings.
- Plug some leaks.
- Fix some free-after-free.
- Fix issue with condition for unavailable metadata, that was
exacerbated with this refactoring.
Comment 19 Carlos Soriano 2016-09-12 11:52:59 UTC
Created attachment 335359 [details] [review]
batch-rename-dialog: major refactoring

This is a refactoring that is difficult to split in several pieces.

This patch implements the following changes:
- Use loops instead of serie of if/elses.
- Use enums and static arrays to define a static serie of tag types
and properties.
- Use enums for ordering and retrieval of properties of tags instead of
searching.
- Merge common code.
- Extract common code in functions.
- Mark for translation some strings.
- Plug some leaks.
- Fix some free-after-free.
- Fix issue with condition for unavailable metadata, that was
exacerbated with this refactoring.
Comment 20 Carlos Soriano 2016-09-12 11:58:26 UTC
Attachment 335345 [details] pushed as a80acee - batch-rename-dialog: use array-for instead of if-elses
Attachment 335346 [details] pushed as ac91c7c - batch-rename-dialog: use 4 spaces for identation
Attachment 335359 [details] pushed as 6669377 - batch-rename-dialog: major refactoring
Comment 21 Carlos Soriano 2016-09-21 18:30:04 UTC
Created attachment 336021 [details] [review]
nautilus-file: copy parameter data in batch renaming

We need to have our own copy and ownership of the parameter data,
if not later on when we do async operations the data might be already
freed.

This was making batch rename crash every time.

This is to follow the common pattern of not transferring the ownership
from caller to called.
Comment 22 Carlos Soriano 2016-09-21 18:30:10 UTC
Created attachment 336022 [details] [review]
nautilus-file: plug leak
Comment 23 Carlos Soriano 2016-09-21 20:30:42 UTC
Attachment 336021 [details] pushed as a508adb - nautilus-file: copy parameter data in batch renaming
Attachment 336022 [details] pushed as 95f7dd7 - nautilus-file: plug leak
Comment 24 Alexandru Pandelea 2016-09-21 21:06:00 UTC
Created attachment 336034 [details] [review]
batch-rename-dialog: Monitor directories for changes

If a new file is added to the directory (or directories, in search),
then the batch rename dialog should check if there will be any conflicts
involving the new added file(s).

If a file is deleted, the dialog should update the list of files by
removing that file from it.

To do this, monitor the directories the files belong to and update the
dialog when a file changes or gets deleted.
Comment 25 Carlos Soriano 2016-09-22 09:31:29 UTC
Created attachment 336066 [details] [review]
batch-rename-dialog: remove unused vars
Comment 26 Carlos Soriano 2016-09-22 09:31:34 UTC
Created attachment 336067 [details] [review]
batch-rename-utilities: fix variable type
Comment 27 Carlos Soriano 2016-09-22 09:31:40 UTC
Created attachment 336068 [details] [review]
batch-rename-utilities: remove unused function
Comment 28 Carlos Soriano 2016-09-22 09:33:55 UTC
Attachment 336066 [details] pushed as d3af002 - batch-rename-dialog: remove unused vars
Attachment 336067 [details] pushed as 1a1e78e - batch-rename-utilities: fix variable type
Attachment 336068 [details] pushed as 46437aa - batch-rename-utilities: remove unused function