vscode-docker icon indicating copy to clipboard operation
vscode-docker copied to clipboard

COPY Command snippets + IntelliSense for selecting the correct file path

Open ucheNkadiCode opened this issue 3 years ago • 19 comments

As the user aims to complete their COPY command, we provide them with

COPY ["hostFolderPath", "containerFolderPath"]

And as they type in their full file paths (example below)

WORKDIR /src COPY ["src/Services/DomainAPI/DomainAPI.csproj", "src/Services/DomainAPI/"]

We provide them the context and tab completion of both their host directory and their container directory (if possible) and take into account their WORKDIR (is the second argument of copy relative to the workdir?).

ucheNkadiCode avatar Dec 03 '20 00:12 ucheNkadiCode

@rcjsuen is this something that is possible with the language server? I don't know much about it so I don't know if it would have access to see the files on the filesystem.

bwateratmsft avatar Dec 03 '20 14:12 bwateratmsft

WORKDIR /src COPY ["src/Services/DomainAPI/DomainAPI.csproj", "src/Services/DomainAPI/"]

We provide them the context and tab completion of both their host directory and their container directory (if possible) and take into account their WORKDIR (is the second argument of copy relative to the workdir?).

@ucheNkadiCode The final argument to the COPY command is an absolute path or a path relative to a previously defined WORKDIR. So in your example, you'll actually have your file in the /src/src/Services/DomainAPI/ folder of your container.

@rcjsuen is this something that is possible with the language server? I don't know much about it so I don't know if it would have access to see the files on the filesystem.

@bwateratmsft We can access the filesystem...but of course, just the local filesystem. Point being, if where you host the language server does not match where your files are then you're not getting anywhere. I think we can try to be smart about the final argument but performing filesystem scans will be a different story.

Also, the build context is not guaranteed to be the directory where the Dockerfile lives so that also complicates things a bit.

rcjsuen avatar Dec 08 '20 20:12 rcjsuen

@rcjsuen we made a change somewhat recently that strongly encourages using workspace mode for the extension (you can make it run in UI mode but only using the setting to force it); so for nearly everyone, the language server should be running in the same place as the files. The context thing is definitely an issue though. It might be pretty difficult or impossible altogether to infer a context other than the Dockerfile's directory...

bwateratmsft avatar Dec 14 '20 19:12 bwateratmsft

For the directory argument, this will require some careful parsing due to backslashes on WORKDIR vs forward slashes on COPY on Windows. On non-Windows systems this is simple enough.

@bwateratmsft I'll see what I can do over the holidays...no promises. :P

rcjsuen avatar Dec 22 '20 23:12 rcjsuen

For the directory argument, this will require some careful parsing due to backslashes on WORKDIR vs forward slashes on COPY on Windows. On non-Windows systems this is simple enough.

@bwateratmsft I'll see what I can do over the holidays...no promises. :P

Hey @rcjsuen, any update on this?

ucheNkadiCode avatar Jan 13 '21 17:01 ucheNkadiCode

Hey @rcjsuen, any update on this?

Sorry, no. I'm trying to wrap my head around the best way to handle this due to the behaviour on Windows. Maybe I'll start by working on the non-Windows behaviour first and worry about Windows support later as a separate enhancement.

rcjsuen avatar Jan 14 '21 13:01 rcjsuen

Hey @rcjsuen, any update on this?

Sorry, no. I'm trying to wrap my head around the best way to handle this due to the behaviour on Windows. Maybe I'll start by working on the non-Windows behaviour first and worry about Windows support later as a separate enhancement.

That would be great! The overwhelming majority of Docker Extensions users use Linux Containers. As for the path IntelliSense, do you think that's possible? If so, this extension looks pretty much as I would expect to help a customer navigate to a file/folder

ucheNkadiCode avatar Jan 15 '21 23:01 ucheNkadiCode

The overwhelming majority of Docker Extensions users use Linux Containers.

That's good to know. There are other editor clients that use the language server but I don't have any usage data there at all...given that I'm just in charge of parsing a file.

As for the path IntelliSense, do you think that's possible?

It is possible if working on the local filesystem but there are performance implications to be had with network filesystems and editors hosted on the web.

rcjsuen avatar Jan 17 '21 14:01 rcjsuen

@ucheNkadiCode I don't think it's that simple of a problem. In order to provide helpful intellisense suggestions for paths, we need to know (with 100% certainty) the Docker build context. If we assume the local folder (or assume/guess anything, for that matter) and it's incorrect, we'd be working against the user.

Build context information is not in the Dockerfile. It's in tasks, default commands, and probably also the user's head. The most realistic thing we could do is start providing path intellisense based on other source paths already in the document--i.e. if they have a COPY ["dir/a.csproj", "dest"], and we see a folder in the tree called dir with a file called a.csproj, we could guess the build context is whatever contains dir.

bwateratmsft avatar Jan 19 '21 13:01 bwateratmsft

@bwateratmsft I believe that some users use "./" to begin their context correct? I know that in compose files, under the "build" section, it's possible to enter "." just to declare the context as "where I am which is the workspace folder". What is the danger of assuming their build context from their host machine is the workspace folder especially if they start their copy command with ./foo/{filename}? What is an example of when the build context is not the workspace folder and there are no commands earlier in the Dockerfile that we can key off of to know?

Also, some users even go the route of putting their FULL system path all the way back from C://Users/Myself/... that wouldn't need a guess right?

ucheNkadiCode avatar Jan 20 '21 18:01 ucheNkadiCode

Compose files do explicitly declare the Docker build context, as you point out, but Dockerfiles do not. Just because they start the path with ./ doesn't mean that the current folder is the build context. (Compose then has this same problem where the "run" context for compose itself is not declared in the compose file)

I'll give an example. In a .NET app, as we currently scaffold it, the root folder (e.g. the solution folder) is the build context, but the Dockerfiles are adjacent their csproj files. Within the Dockerfile, we refer to the csproj with ./subfolder/myproj.csproj, even though myproj.csproj is actually right next to the Dockerfile. If our intellisense suggested ./myproj.csproj, we'd be wrong. The problem is not easy--until they actually run a Docker build command, there's no certainty of what the build context will be.

I don't think putting the full system path as a source is actually supported, but even if it is, it's definitely a bad idea, as it renders your Dockerfile entirely non-portable. It will only work on your system.

bwateratmsft avatar Jan 20 '21 18:01 bwateratmsft

@bwateratmsft I feel like we don't need to immediately worry about the case of if their context is different. It will be important to figure it out over time, but if the user already knows that their context is different than their workspace folder, then they have the option to ignore the suggestions or turn off IntelliSense altogether (which they probably won't do). I suspect most users will have a default context so we should still help those users out. Luckily I see that the feature was already implemented, but my same opinion applies to the Compose volumes section.

@rcjsuen Were you responsible for adding these features?

When I tab complete Copy: image

However, when I write out COPY ["/someFolder", ""] I actually get IntelliSense suggestions for the folder. Since this is also the recommended way to write the command, can you make `COPY ["source", "dest"] the default instead of "COPY source dest"?

image

ucheNkadiCode avatar Feb 01 '21 16:02 ucheNkadiCode

@ucheNkadiCode what OS are you on? I'm not seeing file suggestions on Windows at all.

bwateratmsft avatar Feb 01 '21 17:02 bwateratmsft

@rcjsuen Were you responsible for adding these features?

When I tab complete Copy: image

@ucheNkadiCode Yes, this is from the language server.

However, when I write out COPY ["/someFolder", ""] I actually get IntelliSense suggestions for the folder.

Like @bwateratmsft, I do not see this. I am on macOS.

Since this is also the recommended way to write the command, can you make `COPY ["source", "dest"] the default instead of "COPY source dest"?

Can you link me to the documentation that states this? I know this is mentioned for CMD and ENTRYPOINT in the Dockerfile reference.

rcjsuen avatar Feb 03 '21 13:02 rcjsuen

@rcjsuen we discovered yesterday that the folder/file suggestions @ucheNkadiCode mentioned come from https://marketplace.visualstudio.com/items?itemName=christian-kohler.path-intellisense plugin that is part of his installation ☺️

karolz-ms avatar Feb 03 '21 17:02 karolz-ms

Since this is also the recommended way to write the command, can you make `COPY ["source", "dest"] the default instead of "COPY source dest"?

Can you link me to the documentation that states this? I know this is mentioned for CMD and ENTRYPOINT in the Dockerfile reference.

@rcjsuen The only relevant text I can find is that the COPY ["source", "dest"] format is required if a path contains whitespace (same applies for ADD). Personally I'm fine leaving things how they are, but a reasonable argument can be made that it should be COPY ["source", "dest"], based on whitespaced paths requiring it, and just for the neatness of having the format match the preferred CMD and ENTRYPOINT formats. I don't feel strongly either way. 🤷‍♂️


@rcjsuen we discovered yesterday that the folder/file suggestions @ucheNkadiCode mentioned come from https://marketplace.visualstudio.com/items?itemName=christian-kohler.path-intellisense plugin that is part of his installation ☺️

@karolz-ms @ucheNkadiCode AHA!

bwateratmsft avatar Feb 04 '21 15:02 bwateratmsft

I've tried implementing something simple to support WORKDIR in #2866.

rcjsuen avatar Apr 11 '21 16:04 rcjsuen

@bwateratmsft or @karolz-ms can this be closed down now as there has been some functionality provided at this point?

ucheNkadiCode avatar Jul 16 '21 20:07 ucheNkadiCode

@ucheNkadiCode it is your issue ☺️ I would be fine to close it if you think the support that Remy added is good enough for now.

karolz-ms avatar Jul 16 '21 23:07 karolz-ms

Closing since this is kind of an impossible ask. 😅

bwateratmsft avatar Nov 07 '23 16:11 bwateratmsft