Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recognize Dockerfile.* #1907

Closed
chrisdoherty4 opened this issue Apr 23, 2020 · 10 comments · Fixed by #1908
Closed

Recognize Dockerfile.* #1907

chrisdoherty4 opened this issue Apr 23, 2020 · 10 comments · Fixed by #1908
Milestone

Comments

@chrisdoherty4
Copy link

chrisdoherty4 commented Apr 23, 2020

I'm using vscode-docker v1.1.0 with VSCode 1.44.2 and it doesn't seem to recognise Dockerfile.* files. An issue was raised at microsoft/vscode#38797 (albeit years ago) suggesting it is a feature so I think this is either a bug or there is some setting I haven't found yet that needs turning on?

@bwateratmsft
Copy link
Collaborator

At present this is intentional. Our patterns include Dockerfile, *.dockerfile, Dockerfile.debug, Dockerfile.dev, Dockerfile.develop, Dockerfile.prod.

Our concern with Dockerfile.* is that developers might be using things like: Dockerfile.cs, Dockerfile.js, etc. A C# / JavaScript file isn't a Dockerfile (though, that's not a good name for it either).

You can locally override this behavior with a setting in either user or workspace settings.json:

"files.associations": {
    "Dockerfile.*": "dockerfile"
}

Using this option will allow for both syntax highlighting and the right-click -> Build Image (or, Build Image from command palette) commands.

@chrisdoherty4
Copy link
Author

Understood, I totally see how that can be a problem.

Perhaps this ticket could be expanded to provide a feature that allows users to specify inclusions and exclusions (with a wildcard accepted) where exclusions take priority.

With such a feature you could define include: Dockerfile.* and exclude: Dockerfile.js, Dockerfile.cs etc.

I feel like it's quite common to use the Dockerfile.<descriptive word> approach for file names.

@bwateratmsft
Copy link
Collaborator

Exclusions would require a change from VSCode itself. However, I'm going to try to find out if Dockerfile.cs, etc. is actually a real-world problem. It may be better for those doing that to add a file association making Dockerfile.cs a csharp file.

@chrisdoherty4
Copy link
Author

Understood. The .cs association sounds like a good idea! I wonder if file associations have some precedence ordering? I guess that'll be included in your query.

@bwateratmsft
Copy link
Collaborator

I tried it out.

  1. I changed our file associations (in the extension) to Dockerfile.*
  2. I created Dockerfile.cs
  3. As expected, it was treated as a Dockerfile
  4. I added the following file association:
{
    "files.associations": {
        "*.cs": "csharp"
    }
}
  1. The Dockerfile.cs file was now treated as a C# file.

This behavior is good. I haven't researched yet how widespread this Dockerfile.cs antipattern is, but if it seems to be rare, then it's probably best to change the extension to use Dockerfile.*.

@bwateratmsft
Copy link
Collaborator

bwateratmsft commented Apr 23, 2020

@BigMorty @chrisdoherty4 I looked at VSCode's telemetry some. It's not an exact science; Dockerfiles are detected based on MIME type and all we know is the file extension, not name.

Here's some data on the antipattern usage (last 30 days, limit 100K samples). I looked for any recognizable not-Dockerfile-extension (21 total):

File extension Unique machine count
.yml 4
.dockerignore 3
.js 2
.json 2
.php 2
.yaml 2
.py 1
.js 1
.xml 1
.sh 1
.doc 1
.cs 1

(that .cs might have been us 😄)

In contrast here's the top 20. We cover the first six rows (64102 total), but miss everything else (132 total):

File extension Unique machine count
58904
.dockerfile 3915
.dev 943
.prod 256
.debug 43
.develop 41
.build 20
.base 17
.test 15
.docker 13
.local 10
.template 10
.amd64 9
.aarch64 6
.ci 6
.j2 6
.deploy 5
.df 5
.release 5
.txt 5

Given that users can override the default with a file association so that their not-Dockerfile is treated as a not-Dockerfile, it seems to me that we should change to Dockerfile.*. Beyond that top 20 there were at least 250 more smaller examples, it seems better to me to cater to a larger group (380+) who are using a correct pattern, rather than the smaller group (21) using the antipattern.

@bwateratmsft
Copy link
Collaborator

We discussed this in a meeting and decided to go ahead and make the change, given the data clearly supports it.

@bwateratmsft bwateratmsft self-assigned this Apr 23, 2020
@bwateratmsft bwateratmsft added this to the 1.2.0 milestone Apr 23, 2020
@chrisdoherty4
Copy link
Author

chrisdoherty4 commented Apr 23, 2020

Amazing! love that you brought data to the conversation.
What does your release cycle look like so I have an approximate time on when to expect 1.2.x to release?

@bwateratmsft
Copy link
Collaborator

@chrisdoherty4 Roughly monthly; 1.1.0 just released a few days ago...so about a month? (no promises!)

@bwateratmsft
Copy link
Collaborator

This change is now available in version 1.2.0 of the Docker extension.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants