Copy and paste file/folder in the same place will duplicate the file/folder

[i501378]

Signed-off-by: Lital Avigad lital.avigad@sap.com

What it does

Fixes #8933

Select file/folder and press 'Ctrl+C' and 'Ctrl+V' and the file/folder will be duplicated.

How to test

Mark file/folder and press 'Ctrl+C' and 'Ctrl+V'.

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

[vince-fugnitto]

Thank you for taking care of the issue, I tested and it works well. My only comment is regarding who should actually be responsible for the logic, and I believe it would benefit all copy callers by adding it directly to the service (doCopy).

[vince-fugnitto]

I believe we should move the logic to the actual implementation of fileService.copy, so all clients (callers) who are requesting a similar behavior will not need to duplicate the logic. For example, the doMoveCopy is handling the _copy like you've done:

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

Lines 1120 to 1125 in 5db308f

	// 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);
	}

[kittaakos]

I believe we should move the logic to the actual implementation of fileService.copy

I do not think so. If you debug into the VS Code's FileService, that we copied, you will see the target URI is the expected target URI (file:///path/to/file copy or file:///path/to/file copy 2). Code does not do any more magic just copies/moves the file. We should do the same since we copied the code and we do not want to diverge from it.

By the way, I proposed something similar here

[i501378]

Hi @kittaakos and @vince-fugnitto ,

I am confused, can you tell me what should I do so this PR will be approved?

Regards,

Lital.

[kittaakos]

I am confused, can you tell me what should I do so this PR will be approved?

I am a little bit off from the Theia loops recently so I am just suggesting here, it's up to you how you implement it. I think this is the fix we need: #9037 (comment)

The FileService API should not change the target URI when moving/copying. Imagine you have a Node.js call:

fs.copyFile('/path/to/my/file', '/path/to/desired/destination', cb)

And eventually, it will copy my file to /path/to/desired/destination_copy just because /path/to/desired/destination already exists. No, the FileService should fail and not modify the destination.

[vince-fugnitto]

@i501378 thank you for the changes 👍 it seems as though the initial issue #8933 is fixed by #9037 so it should be safe to close.