https://bugzilla.xfce.org/show_bug.cgi?id=16686
--- Comment #9 from alexxcons <[email protected]> --- (In reply to Cyrille Pontvieux from comment #8) > Created attachment 9772 [details] > 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) -- You are receiving this mail because: You are the assignee for the bug. _______________________________________________ Xfce-bugs mailing list [email protected] https://mail.xfce.org/mailman/listinfo/xfce-bugs
