Created attachment 9713 patched based on pause-copy branch Allow to rename conflicted file names when copy/move files. The dialog don't allow fancy editing but relies on the already-working suffix `(copy X)` appended when copying a single file in the same directory. To allow this, I needed to add two more possible answers to `THUNAR_JOB_RESPONSE`: `RENAME` and `RENAME_ALL`. The attached patched is based on my previous patch/bug (https://bugzilla.xfce.org/show_bug.cgi?id=16685) The commit can also be found in a git branch (url attached to this bug).
Created attachment 9717 patch based on pause-copy branch
Possibly a dup: Bug #11407 ( I did not check in detail)
Bug #11407 and the Dolphin implementation seems interesting, unfortunately I didn't take that path, but a simpler one, with no filename dialog implied. I'm still not sure on how to handle this for more than one file. I mean it could be cumbersome to have a dialog asking for a name whenever a conflict filename exists on the target directory. Maybe it's desirable, maybe not. If the more complicated (Dolphin-way) dialog is wanted, I could improve this patch, but I think some UI design mockups should be done first, just for everybody to be sure of what is wanted.
I've given this a quick test and it seems to work as intended with no problems. I think it's good to have the option to rename rather than just replace or skip/cancel. I think it might be better to have a renamer dialog here rather than just appending the "copy" suffix. My reasoning is as follows: when you copy a file into the same directory, the new file is a copy of the original, so you have "file_a" and "file_a (copy 1)" where "file_a (copy 1)" really is a copy of "file_a". But if you are pasting some "file_a" from another directory into a directory already containing a "file_a", then there is no guarantee that the two files will be the same, and if you rename the pasted file to "file_a (copy 1)" then it might not be the case that the file called "file_a (copy 1)" is a copy of "file_a". Maybe I am being too pedantic here though... For more than one file, perhaps you should open the bulk renamer? Also, there is now a new widget in libxfce4ui called XfceFilenameInput that is specifically for filename input and should make implementing a renamer dialog a bit easier. It is not used in thunar yet but I am planning to fix this soon!
You're right, it's not really a “copy” and so it may be confusing to use the same wording. We can use something different, but a more complex solution with a filename input *could* be better. I'm ok to do this work (and try XfceFilenameInput) but I'm not really sure of the workflow. We can design simple mockups on draw.io for instance.
(In reply to Cyrille Pontvieux from comment #5) On second thoughts, maybe it's just the word "copy" that is bothering me here. Maybe in this patch one could just rename "file_a" to "file_a (1)", as that does not say it is a copy, just another version. Then perhaps you could (as a second patch) work on making a more interactive renaming process? In any case, these are just my thoughts. It's probably best to wait to see what alexxcons thinks of this.
(In reply to Cyrille Pontvieux from comment #1) > Created attachment 9717 > patch based on pause-copy branch Patch does not apply on current master .. possibly you need to rebase on the latest changes for the "pause" fix ? (or is it me doing something wrong ?) (In reply to Cyrille Pontvieux from comment #3) > I'm still not sure on how to handle this for more than one file. I mean it could be cumbersome to have a dialog asking for a > name whenever a conflict filename exists on the target directory. Maybe it's desirable, maybe not. > If the more complicated (Dolphin-way) dialog is wanted, I could improve this patch, but I think some UI design mockups > should be done first, just for everybody to be sure of what is wanted. As far as I can see, as well dolphin asks per file .. so no problem on that. Dolphin has some "compare pictures" feature .. yes, that's nice, though lets start with your approach, which already should improve the current situation. (In reply to Reuben Green from comment #4) > For more than one file, perhaps you should open the bulk renamer? That might be confusing for people which did not use the bulk renamer till now, and it requires knowledge on the number of possible conflicts in advance. For now I would keep it simple, and just open one dialog per conflict ... since it is possible to press "cancel all", I guess we are fine. (In reply to Reuben Green from comment #6) > On second thoughts, maybe it's just the word "copy" that is bothering me here. > Maybe in this patch one could just rename "file_a" to "file_a (1)", as that does not say it is a copy, just another version. Afaik that's just the thunar default (or does the patch add extra-code to do like that ?) If you press ctrl+c + ctrl+v on a single file, you will get the same result. So as a first step, re-using the default would be fine for me. (Though we could generally change that default to use "file_a (1)" , etc. in a different bug, if you think it would be worth it) As you proposed, I guess using the rename dialog would fir better .. would it be hard to do ? We already have "thunar_dialogs_show_rename_file" which could be used. If it would require alot of code-changes, I would go with the "copy of" for this bug, and introduce "thunar_dialogs_show_rename_file" in a separate bug. (In reply to Reuben Green from comment #6) > Also, there is now a new widget in libxfce4ui called XfceFilenameInput that is > specifically for filename input and should make implementing a renamer > dialog a bit easier. It is not used in thunar yet but I am planning to fix So far I did not test the patch, though wouldnt it be fine to just use "thunar_dialogs_show_rename_file" here ? I guess only the internals of "thunar_dialogs_show_rename_file" need to be changed in order to use XfceFilenameInput.
Created attachment 9772 format-patch to apply on latest master For rename, I used the already written code about appending " (copy X)" and its related translations. So I keep it simple in this commit. And let's improve it further in another commit/issue. Thanks you two for taking time to look at this.
(In reply to Cyrille Pontvieux from comment #8) > Created attachment 9772 > format-patch to apply on latest master > > For rename, I used the already written code about appending " (copy X)" and > its related translations. > > So I keep it simple in this commit. And let's improve it further in another > commit/issue. > > Thanks you two for taking time to look at this. Patch works great for me, thanks ! Reading the code: - Possibly g_object_unref (renamed_file); is missing in thunar_transfer_job_copy_file (like for duplicate_file). I think you can use the scope of the loop for the variable, like for duplicate_file. ... at least something is fishy there. I dont understand that part: > if (err == NULL) > target_file = renamed_file; Overwriting an argument usually is not what you want to have ... besides that, target_file is not used any more later in that method. - You can set "n_rename = 0;" directly on variable declaration, no need for an extra line > thunar-transfer-job.c:1048 "while (try_again)" Why you need to loop here, wheras before there was no loop. What would be a use-case which uses that additional loop ? There is one thing bothering me when reading the code, not directly on your patch, but related: > THUNAR_JOB_RESPONSE_YES / THUNAR_JOB_RESPONSE_YES_ALL As far as I can see, it would make much more sense to have THUNAR_JOB_RESPONSE_REPLACE / THUNAR_JOB_RESPONSE_REPLACE_ALL for our job in order to reflect what the buttons do ... that would make the code much simpler to understand. Since THUNAR_JOB_RESPONSE_YES is as well used in "thunar_job_ask_create", I would add addtional enum-values instead of renaming of existing enum-values. Same for THUNAR_JOB_RESPONSE_NO / THUNAR_JOB_RESPONSE_NO_ALL which IMO should be THUNAR_JOB_RESPONSE_SKIP / THUNAR_JOB_RESPONSE_SKIP_ALL You think such a rename would be reasonable ? It should be done in a separate patch (I can do so later on .. though if you have time, feel free to start over)
> Reading the code: > - Possibly g_object_unref (renamed_file); is missing in > thunar_transfer_job_copy_file (like for duplicate_file). > I think you can use the scope of the loop for the variable, like for > duplicate_file. > ... at least something is fishy there. I dont understand that part: > > if (err == NULL) > > target_file = renamed_file; > Overwriting an argument usually is not what you want to have ... besides > that, target_file is not used any more later in that method. Well actually, a g_object_unref is required, but on target_file. I replace target_file here, because it is use later on the loop for the next attempt to copy the file. I know that replacing an argument is not usually a good idea, but I'm not sure how to do it without heavily modify the function. > - You can set "n_rename = 0;" directly on variable declaration, no need for > an extra line Yes thanks > > thunar-transfer-job.c:1048 "while (try_again)" > Why you need to loop here, wheras before there was no loop. What would be a > use-case which uses that additional loop ? I tried not to modify too much this function. So when the new file name is defined, the move function have already been tried and I didn't want to copy the code (I like the DRY, Don't Repeat Yourself, approach). But maybe here was not the best way to do it. And if you don't immediately understand the code, that means it's a bad-written code. I will propose a easier solution. > There is one thing bothering me when reading the code, not directly on your > patch, but related: > > THUNAR_JOB_RESPONSE_YES / THUNAR_JOB_RESPONSE_YES_ALL > > As far as I can see, it would make much more sense to have > THUNAR_JOB_RESPONSE_REPLACE / THUNAR_JOB_RESPONSE_REPLACE_ALL for our job in > order to reflect what the buttons do ... that would make the code much > simpler to understand. > Since THUNAR_JOB_RESPONSE_YES is as well used in "thunar_job_ask_create", I > would add addtional enum-values instead of renaming of existing enum-values. > > Same for THUNAR_JOB_RESPONSE_NO / THUNAR_JOB_RESPONSE_NO_ALL which IMO > should be THUNAR_JOB_RESPONSE_SKIP / THUNAR_JOB_RESPONSE_SKIP_ALL > You think such a rename would be reasonable ? Yes, I think it will clarify the code > It should be done in a separate patch (I can do so later on .. though if you > have time, feel free to start over) I'll try, so I will propose two patches, one patch with improved corrections and one patch for the response enum additions. I will propably try to use xfce gitlab where I already cloned the thunar repo instead of using bugzilla attachements if it's ok for you.
First merge request: https://gitlab.xfce.org/xfce/thunar/-/merge_requests/2
[Here](https://gitlab.xfce.org/xfce/thunar/-/merge_requests/3) is the merge request for job_response renames
Nice ! I did not know that merge requests already work there :) .. I'll take a look !
Cyrille Pontvieux referenced this bugreport in commit 8cea485716c1304d790fa8a7677f82acc5c47e98 Split "thunar_transfer_job_execute" in multiple simpler functions to keep the code human readable, and as preperation for Bug #16686 https://gitlab.xfce.org/xfce/thunar/commit/8cea485716c1304d790fa8a7677f82acc5c47e98
Cyrille Pontvieux referenced this bugreport in commit 544dad41c6b6eee640fed9fece3f22a856342372 Add THUNAR_JOB_RESPONSE_REPLACE and THUNAR_JOB_RESPONSE_SKIP to keep the code human readable, and as preperation for Bug #16686 https://gitlab.xfce.org/xfce/thunar/commit/544dad41c6b6eee640fed9fece3f22a856342372
Cyrille Pontvieux referenced this bugreport in commit 500026ac163dfb5f93ce187b3a5b29cca4582a34 Option to rename a file when existing copy conflicts (Bug #16686) https://gitlab.xfce.org/xfce/thunar/commit/500026ac163dfb5f93ce187b3a5b29cca4582a34
Opened Bug #16794 for "Ask for a new name instead of (Copy 1)" Thanks alot Cyrille, for your contribution ! As well thanks to Reuben for testing the patches !
.. pushed to master, will be released in thunar 1.9.0
*** Bug 10751 has been marked as a duplicate of this bug. ***