Fix for #8933 - Copy and paste file/folder in the same place will duplicate the file/folder

[ronabet]

Signed-off-by: Ron Nabet ron.nabet@sap.com

What it does

Fixes #8933
Select file/folder and press 'Ctrl+C' and 'Ctrl+V' and the file/folder will be duplicated instead of an error.

How to test

Mark file/folder and press 'Ctrl+C' and 'Ctrl+V'.
Copy a file/folder and paste under the same folder, the file will be created with "_copy" at the end of the filename.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[kittaakos]

Please do not change FileService, it was copied from VS Code. In fact, I'd move out the Theia-specific code from the FileService when generating a non-conflicting basename. Please change.

Here is a working proposal, it's up to you how you implement it, but we won't hack the FileService. Thank you for your understanding.

diff --git a/packages/filesystem/src/browser/file-service.ts b/packages/filesystem/src/browser/file-service.ts
index 89493b5e5..2f3b3918b 100644
--- a/packages/filesystem/src/browser/file-service.ts
+++ b/packages/filesystem/src/browser/file-service.ts
@@ -1117,13 +1117,6 @@ export class FileService {
         // validation
         const { exists, isSameResourceWithDifferentPathCase } = await this.doValidateMoveCopy(sourceProvider, source, targetProvider, target, mode, overwrite);
 
-        // if target exists get valid target
-        if (exists && !overwrite) {
-            const parent = await this.resolve(target.parent);
-            const name = target.path.name + '_copy';
-            target = FileSystemUtils.generateUniqueResourceURI(target.parent, parent, name, target.path.ext);
-        }
-
         // delete as needed (unless target is same resource with different path case)
         if (exists && !isSameResourceWithDifferentPathCase && overwrite) {
             await this.delete(target, { recursive: true });
diff --git a/packages/filesystem/src/browser/file-tree/file-tree-model.ts b/packages/filesystem/src/browser/file-tree/file-tree-model.ts
index 1f8440e41..38a128c25 100644
--- a/packages/filesystem/src/browser/file-tree/file-tree-model.ts
+++ b/packages/filesystem/src/browser/file-tree/file-tree-model.ts
@@ -24,6 +24,7 @@ import { FileService } from '../file-service';
 import { FileOperationError, FileOperationResult, FileChangesEvent, FileChangeType, FileChange, FileOperation, FileOperationEvent } from '../../common/files';
 import { MessageService } from '@theia/core/lib/common/message-service';
 import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
+import { FileSystemUtils } from '../../common';
 
 @injectable()
 export class FileTreeModel extends TreeModelImpl implements LocationService {
@@ -148,8 +149,14 @@ export class FileTreeModel extends TreeModelImpl implements LocationService {
     }
 
     async copy(source: URI, target: Readonly<FileStatNode>): Promise<URI> {
-        const targetUri = target.uri.resolve(source.path.base);
+        const baseUri = (DirNode.is(target) ? target : ((target.parent || this.root!) as Readonly<FileStatNode>)).fileStat.resource;
+        let targetUri = baseUri.resolve(source.path.base);
         try {
+            if (source.toString() === targetUri.toString()) {
+                const parent = await this.fileService.resolve(targetUri.parent);
+                const name = targetUri.path.name + '_copy';
+                targetUri = FileSystemUtils.generateUniqueResourceURI(targetUri.parent, parent, name, targetUri.path.ext);
+            }
             await this.fileService.copy(source, targetUri);
         } catch (e) {
             this.messageService.error(e.message);

[kittaakos]

First off, your PR is not the same as my proposal. Secondly, please squash your commits. Finally, I won't review and accept my code, it's against our review principles. Please find someone else for the review. Thank you!

[amiramw]

@ronabet when I try to Ctrl+C and then Ctrl+V on a folder I get infinite nested libraries. In your description it says folder copy should be fixed as well in this PR.

[ronabet]

@ronabet when I try to Ctrl+C and then Ctrl+V on a folder I get infinite nested libraries. In your description it says folder copy should be fixed as well in this PR.

Fixed

[amiramw]

@ronabet @i501378 is it intentional that now #8778 doesn't work? Instead of pasting the file with suffix I see now error notification "File at target already exists".

[ronabet]

@ronabet @i501378 is it intentional that now #8778 doesn't work? Instead of pasting the file with suffix I see now error notification "File at target already exists".

I fixed it.

[kittaakos]

FYI: you have merged a PR that was different than the expected one. The FileService contains the same logic you have added.

theia/packages/filesystem/src/browser/file-service.ts

Lines 1121 to 1125 in b972e2b

	if (exists && !overwrite) {
	const parent = await this.resolve(target.parent);
	const name = target.path.name + '_copy';
	target = FileSystemUtils.generateUniqueResourceURI(target.parent, parent, name, target.path.ext);
	}

Related: #8940 (comment)

[amiramw]

Oh, sorry. I think when @ronabet removed the code from file-service then #8778 flow got broken.
How do you suggest to proceed?

[kittaakos]

How do you suggest to proceed?

OK, no worries. I open a follow-up and will take care of it.