Support copy name of active file

[iliashkolyar]

Supports #54.
Since VSCode natively supports "Copy Path Of Active File" and "Copy Relative Path Of Active File", I only added "Copy Name Of Active File" which was missing.

[iliashkolyar]

It seems that the test fails as xclip isn't installed by default on Linux machines.
Is it possible to add it to the CI running machine or should I disable this test for this OS?

[sleistner]

It seems that the test fails as xclip isn't installed by default on Linux machines.
Is it possible to add it to the CI running machine or should I disable this test for this OS?

Does that mean the command would fail on Linux machines?

[iliashkolyar]

Yes, on Linux systems that do not have the xclip package installed.
I currently suppress the error (if triggers) in the test and in the Controller.
Is there a way to show the client the error (for example in the VSCode console) so that he'll install xclip on his machine?

[sleistner]

Do you think there's a way to use the VSCode copy/paste system which is used internally?

[iliashkolyar]

Do you think there's a way to use the VSCode copy/paste system which is used internally?

As you can see in the following posts, it seems that vscode are not exposing nor intend to expose their clipboard API:
microsoft/vscode#10933 (comment)
microsoft/vscode#4972 (comment)

When people asked them to support the "Copy File Name", they inclined that an extension should be used.

[sleistner]

OK, fair enough.

Have you seen the vulnerability message from snyk?

sync-exec is an fs.execSync replacement for node <0.12.

Affected versions of this package use tmp directories in an insecure way. 
The file to create will allways return true, regardess if the directory already exists and/or belongs to another user. 
Any user on the server may read the contents of this tmp file and may expose confidential information to an attacker.

[iliashkolyar]

Replaced copy-paste package with a fork that removed the vulnerability (See the following thread for more details).

[sleistner]

What do you think about the following architecture:

1. Implement name as method in FileItem
2. CopyFileNameController extends BaseFileController
3. CopyFileNameController#execute returns Promise<FileItem>
4. CopyFileNameCommand extends BaseCommand

[sleistner]

Maybe something like this.

export class CopyFileNameCommand extends BaseCommand {

    [...]

    public async execute(uri?: Uri) {
        const sourcePath: string = this.controller.sourcePath;
        if (!sourcePath) {
            return;
        }
        const fileItem = new FileItem(sourcePath);
        return this.controller.execute({ fileItem });
    }

}

export interface IFileController {
    readonly sourcePath: string;
    [...]
}

[iliashkolyar]

I was thinking that a new interface for different copy operations (incase we'd like to support other copy types) would be better than overloading the fileItem with Copy related fields.

But if you feel that this is a better approach - no problem.

[sleistner]

I think it would make sense to keep the api consistent. Also making name a getter method of FileItem supports the idea of separation of concerns.
Appreciated if you don't mind changing it.

And thank you very much for contributing, great work!

[iliashkolyar]

Up until now, sourcePath was a getter that was called on demand by the controllers.

If I understood you correctly, here you suggest that copyFileNameCommand will access this.controller.sourcePath, and that the sourcePath parameter will be publicly available on the BaseFileController.

What I don't understand is where will it be populated?
You don't have the relevant context when the controller is created.

I can stick with the util.getSourcePath() approach so it could be retrieved from both Messages and Controllers.

What do you think?

[sleistner]

I meant to extend the IFileController interface with

readonly sourcePath: string;

and change BaseFileController

-    protected get sourcePath(): string {
+    public get sourcePath(): string {

That way you don't need util at all.