Community
Participate
Working Groups
Yes, I add this issue though it contradicts to issue #3985 and issue #33071. I really wonder what the advantage of this prefix should be. I never just wanted to create a copy where the name is not important like a backup. Whenever I copy a file I want to specify a new name. And there it is really annoying to remove the prefix over and over again. If this is now different for other people (otherwise it would probably not have been added this way) it might make sense to make this prefix configurable.
I agree, 'CopyOf' is mostly useless. But this has been done for consistency with the platform. We won't change that anymore. But I see this as a quite minor issue and don't think this justifies a new preference.
And changing it on the platform level will most probably neither happen? (I only added it to JDT > UI as the other two issues concerning were classified the same way. Actually I want it to be changed for platform too as the "Copy of " prefix for non-Java files is also annoying.)
I completely agree. The only time I leave the prefix is when I forget to remove it. If people really find this useful, how about this: 1. Start without the prefix 2. If they click OK or hit [Enter] and the name conflicts, A. Add the prefix B. Show a warning If you want the prefix you can just hit [Enter] twice. For the 99.999% of the cases where you don't want the prefix, you no longer have to remove it. This one change alone could probably save 527.138 developer hours, 24.58 kWh, and 19.74 Joules of frustration energy build-up per year.
The prefix is indeed bad. We should just append "2" to the existing name, unless the name already ends in a number. In that case, we should increment the number by 1 until the resulting name is available.
I've also reopened bug 200444.
I'm working at getting a fix proposal for this one. Hope to have more to share soon !
Created attachment 243164 [details] Fix proposal The new test covers the change in the sense it's red without the fix and green with it. But I was wondering if there's also need for finer grain testing for private method computeNewName() I added in ReorgPolicyFactory; for now I tested it separately.
(In reply to Yves Joan from comment #7) > Created attachment 243164 [details] [diff] > Fix proposal Please add an explicit CoO sign-off comment with the patch. See: http://wiki.eclipse.org/Development_Resources/Contributing_via_Git#via_Bugzilla Review comments: - Go through the contributors guide and update the copyright header of all the files as described in: https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Coding_Conventions - Update the patch based on current master branch. - #computeNewName(String): In case of IResource (and IPackageFragment, if we use this method to copy/paste packages also), the string after last dot in the name need not always be an extension. For example a folder, file with no extension etc that have a dot in the name. Hence, in #computeNewName(String), we cannot always ignore the last segment. => A solution could be to pass only the name without extension to #computeNewName(String). Extension should be removed before passing the name only if it is called from #createNewName(ICompilationUnit, IPackageFragment) or #createNewName(IResource, IContainer) where the resource is an instance of IFile. In the latter, if the resource is a file with no extension and multiple segments, should we try to handle that differently? Dani / Markus, please suggest. - The current behavior while copy/pasting a package adds a suffix ".copy" to the package name, which is not bad. Dani / Markus, should we update that also to append / increment the number at the end of the package name? - Regarding tests: You can update other test methods also having *same* (not ending with "cancel"). In the test you added, there is no Copy_File_to_Itself possible. In such a case, the destination is the containing parent, not the same file. You can add some similar test methods handling conflicts that will also test the result from #computeNewName(String).
Partial answer to <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=151668#c8">comment 8</a> : This contribution complies with http://www.eclipse.org/legal/CoO.php
Created attachment 243874 [details] Patch proposal v2
(In reply to Noopur Gupta from comment #8) In the patch version #2 and compared to previous one: -headers are fixed as per coding conventions -patch is based on current master branch - computeNewName() acquired a new parameter, the resource type. It allows to detect if we're in the IFile case or not then the folder case is now properly managed. But still if we face a file name with several segments we continue to ignore the last one. - on tests front: - I've added testCopy_folder_to_same_container_2 that covers at the same time a name with multiple segments (folder.name.with.segments) and a conflicting suggested name: because folder.name.with.segments2 already exists then the quickfix must suggest folder.name.with.segments3. - I've renamed testCopy_File_to_Itself_Conflict in testCopy_File_to_Itself_2. - about (*same* && ! *cancel*) I've updated testCopy_folder_to_same_container.
(In reply to Noopur Gupta from comment #8) Discussed with Dani and Markus. > In the latter, if the resource is a file with no extension and multiple > segments, should we try to handle that differently? No special handling is required. The string after last dot can be considered as the extension. > - The current behavior while copy/pasting a package adds a suffix ".copy" to > the package name, which is not bad. No change is required in the existing behavior. We should also take care of the following: - Files like README (file with no extension; should suggest README2) - Files like .gitignore, .project (should suggest .gitignore2) - Folders like .settings (should suggest .settings2) I did not look at the updated patch yet as it needs an update for files like .gitignore. Yves, please handle that and attach the new patch.
Created attachment 244110 [details] Patch proposal v3
(In reply to Noopur Gupta from comment #12) > I did not look at the updated patch yet as it needs an update for files like > .gitignore. Yves, please handle that and attach the new patch. Done: the patch proposal v3 takes into account the extra cases you listed.
Thanks. Patch v3 looks good. The test methods #copy_File_to_Itself_2_impl and #copy_folder_to_same_container_2_impl can be renamed by removing "2" from the name. I will do that before committing the patch for Mars.
Released as: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=fe71861b8ec909a0117c2711c747fa35089d00fe
Please don't add unnecessary 'final' modifiers for local variables. They make the code hard to read. In contrast, final fields are fine, since they spare readers from doing lengthy investigations. If the occurrences of a local variable don't fit on one screen, then the method should be refactored. Fixed that and removed the unnecessary creation of an Integer object: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=ad9f28d7525b62a0337f9c78c48a7c25c677e243
