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

DocumentFilter does not work for dockerfiles #878

Closed
peterj opened this issue Dec 1, 2015 · 38 comments
Closed

DocumentFilter does not work for dockerfiles #878

peterj opened this issue Dec 1, 2015 · 38 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug
Milestone

Comments

@peterj
Copy link

peterj commented Dec 1, 2015

We want our docker extension to support yml and docker files when they are named like ‘docker-compose-dev.yml’ or ‘dockerfile-dev’ etc. Using the DocumentFilter pattern **/docker-compose*.yml I was able to get this to work, however DocumentFilter **/dockerfile* does not work – I don’t even get syntax coloring.

@jrieken
Copy link
Member

jrieken commented Dec 2, 2015

@PeterJausovec Just to clarify, you have a filter like this { pattern: '**/docker-compose*.yml' }?

@jrieken jrieken added the info-needed Issue requires more information from poster label Dec 2, 2015
@peterj
Copy link
Author

peterj commented Dec 2, 2015

Yes, filter like the one below works fine:
vscode.DocumentFilter = { language: 'yaml', scheme: 'file', pattern: '**/docker-compose*.yml' };

However, if I try to use a filter for Dockerfile - e.g. **/dockerfile*, it does not work.

@jrieken
Copy link
Member

jrieken commented Dec 2, 2015

I'd say that's because somepath/dockerfile doesn't match the pattern **/docker-compose*.yml or am I missing something?

The properties of the document filter must all match so that the filter matches (logical AND). You can either change the pattern (maybe **/docker*.yml) or use make up the selector from an array of two filers like [{pattern: '**docker-compose*.yml'}, { pattern: '**/dockerfile*}] because then just one must match (logical OR)

@jrieken jrieken closed this as completed Dec 2, 2015
@jrieken jrieken removed the info-needed Issue requires more information from poster label Dec 2, 2015
@peterj
Copy link
Author

peterj commented Dec 2, 2015

Yes, I know it doesn't - I have a separate document filter that's for dockerfile only and its pattern is '**/dockerfile* and files like dockerfile, blah/dockerfile-dev, etc. are not recognized by the pattern.

@peterj
Copy link
Author

peterj commented Dec 2, 2015

Check the file here: https://github.com/Microsoft/vscode-docker/blob/master/dockerExtension.ts. If I add the pattern to const DOCKERFILE_MODE_ID: vscode.DocumentFilter = { language: 'dockerfile', scheme: 'file' };, dockerfiles are not recognized.

@jrieken jrieken reopened this Dec 2, 2015
@jrieken jrieken assigned bpasero and unassigned jrieken Dec 2, 2015
@jrieken
Copy link
Member

jrieken commented Dec 2, 2015

This seems to be a problem with our glob matcher

@bpasero bpasero added the bug Issue identified by VS Code Team member as probable bug label Dec 2, 2015
@bpasero bpasero added this to the Dec 2015 milestone Dec 2, 2015
@bpasero
Copy link
Member

bpasero commented Dec 2, 2015

@jrieken I don't see how a pattern of **/dockerfile* can match a file docker-compose-dev.yml?

@bpasero bpasero removed their assignment Dec 2, 2015
@peterj
Copy link
Author

peterj commented Dec 2, 2015

I am not trying to match docker file to the docker compose pattern. Check the source file I referenced above.

@jrieken
Copy link
Member

jrieken commented Dec 2, 2015

Sorry, I am not getting it. Can you point to the exact line in that file?

@peterj
Copy link
Author

peterj commented Dec 2, 2015

For docker-compose files I am using the pattern below. This works fine.
Line 20: const YAML_MODE_ID: vscode.DocumentFilter = { language: 'yaml', scheme: 'file', pattern: '**/docker-compose*.yml' };

I was trying to use the pattern **/dockerfile* and put it in line 15:
const DOCKERFILE_MODE_ID: vscode.DocumentFilter = { language: 'dockerfile', scheme: 'file' };

However, using the pattern **/dockerfile* in line 15 for the dockerfile does not work.

@jrieken
Copy link
Member

jrieken commented Dec 2, 2015

@PeterJausovec please confirm these steps and will in the uri of your document

  • have a document selector like this { language: 'dockerfile', scheme: 'file', pattern: '**/dockerfile*'}
  • have a TextDocument with a uri ????
  • -> selector doesn't match?

@peterj
Copy link
Author

peterj commented Dec 2, 2015

I added this: vscode.DocumentFilter = { language: 'dockerfile', scheme: 'file', pattern: '**/dockerfile*' };, I open a file named Dockerfile - I get syntax coloring and all works good.

If I open a file named DockerfileBlah or Dockerfile.test I do not get any syntax coloring.

@jrieken
Copy link
Member

jrieken commented Dec 2, 2015

so it does seems to be glob related. please try using just this vscode.DocumentFilter = { pattern: '**/dockerfile*' }

@peterj
Copy link
Author

peterj commented Dec 2, 2015

I tried const DOCKERFILE_MODE_ID: vscode.DocumentFilter = { pattern: '**/dockerfile*' }; and I get the exact same behavior: file named Dockerfile works, but DockerfileTest doesn't.

@jrieken
Copy link
Member

jrieken commented Dec 2, 2015

ok. thanks for the heads-up

@bpasero
Copy link
Member

bpasero commented Dec 2, 2015

Ok I get it, our glob matcher is case sensitive. There is currently no option to test ignoring case. I think the easiest would be to lowercase the file name when passing into the glob call @jrieken

@bpasero bpasero assigned jrieken and unassigned bpasero Dec 3, 2015
@jrieken
Copy link
Member

jrieken commented Dec 3, 2015

I am unsure if making stuff lower-case is desirable...

@bpasero
Copy link
Member

bpasero commented Dec 3, 2015

If a user can provide multiple document filters for the same kind, the solution is to just provide as many glob patterns as needed to catch the casing too.

@jrieken
Copy link
Member

jrieken commented Dec 3, 2015

yeah. you can do [{pattern: '**/lowercase'}, {pattern: '**/Uppercase'}] tho it would be pita to provide all possible permutations. @PeterJausovec is your scenario actually such that case matter and is it common that the file must match on name (like a package.json, tsconfig.json) but is given varying cases?

@bpasero
Copy link
Member

bpasero commented Dec 3, 2015

I can add a flag to glob to ignore casing, since its just a RegEx in the end it should be easy.

@bpasero
Copy link
Member

bpasero commented Dec 3, 2015

(that is, it would be an option, I dont want to enable this for any glob)

@jrieken
Copy link
Member

jrieken commented Dec 3, 2015

unless the flag is in the glob expression itself, it will be the same as me lower-casing the string first I guess

@bpasero
Copy link
Member

bpasero commented Dec 3, 2015

Right, since glob patterns are not RegExp syntax, there is no syntax from what I remember to form a case-insensitive glob. Would it make sense to allow to set this flag from the side of the extension?

{pattern: '**/lowercase', ignoreCase: true}

@peterj
Copy link
Author

peterj commented Dec 3, 2015

I would say case doesn't matter; since Docker users will probably have more than 1 dockerfile, our idea was to provide the syntax coloring and all that magic for files that are named dockerfile*.

@jrieken jrieken modified the milestones: Backlog, Dec 2015 Dec 14, 2015
@jrieken
Copy link
Member

jrieken commented Dec 14, 2015

Needs more thinking. Tend towards not doing this.

@SteveLasker
Copy link

We've updated our yo docker tools to use docker.debug /docker.release to enable developers to compose up multiple environments. This means our scaffolding is generating assets that hide the docker editor features. Is it possible to get this in the January 2016 milestone?

@egamma
Copy link
Member

egamma commented Dec 18, 2015

CC @chrisdias

@jrieken
Copy link
Member

jrieken commented Dec 22, 2015

@SteveLasker, @PeterJausovec Is it that you guys want to contribute a language service for docker files or do you want to add specific features for certain docker files - those that match a name pattern? Can you provide some samples, like filename -> [feature set] etc?

@peterj
Copy link
Author

peterj commented Dec 23, 2015

We own the docker extension for VS Code (https://github.com/microsoft/vscode-docker) and as I mentioned earlier, we would like to provide support for files that match the pattern dockerfile* (e.g. Dockerfile.dev, Dockerfile-blah, etc.). We want all files that match the pattern above to have the same features as the current Dockerfile pattern has. For example, if I open a dockerfile in VS Code today, I get syntax highlighting and all that magic. However, if I open a file named dockerfile.debug I don't get any of that.

@jrieken
Copy link
Member

jrieken commented Jan 4, 2016

So what you guys actually want is to be able to map more filenames to the language dockerfile such that (this)[https://github.com/Microsoft/vscode-docker/blob/master/dockerExtension.ts#L15] selector works for more filenames and such that the grammar works for more filenames.

@jrieken
Copy link
Member

jrieken commented Jan 4, 2016

That is then not the document filter, but the language mapping as defined in package.json (https://code.visualstudio.com/docs/extensionAPI/extension-points#_contributeslanguages)

@jrieken
Copy link
Member

jrieken commented Jan 4, 2016

@alexandrudima, @bpasero Do we already support some sort of globbing in the filename-to-language mapping?

@SteveLasker
Copy link

Just wanted to check on the status of this. We'd really like to close this item out and promote the extension.

Sent from my phone

On Jan 4, 2016, at 1:04 AM, Johannes Rieken notifications@github.com wrote:

@alexandrudima, @bpasero Do we already support some sort of globbing in the filename-to-language mapping?


Reply to this email directly or view it on GitHub.

@jrieken
Copy link
Member

jrieken commented Jan 6, 2016

@SteveLasker I am still waiting for answers regarding adding a language vs a language feature (#878 (comment))

@SteveLasker
Copy link

Are you looking for input from us or the underlying implementation?

Sent from my phone

On Jan 6, 2016, at 8:16 AM, Johannes Rieken notifications@github.com wrote:

@SteveLasker I am still waiting for answers regarding adding a language vs a language feature (#878 (comment))


Reply to this email directly or view it on GitHub.

@jrieken
Copy link
Member

jrieken commented Jan 6, 2016

  1. You want to contribute a file pattern to language mapping similar to *.js -> JavaScript (https://code.visualstudio.com/docs/extensionAPI/extension-points#_contributeslanguages) but need some more powerful like glob-patterns
  2. You want to contribute a language feature like IntelliSense using a DocumentFilter independent of the language of a document but purely based on a glob-pattern, like { pattern: "*.js"} instead of {language: "JavaScript"}.

@SteveLasker, @PeterJausovec Please answer (1) or (2).

@bpasero
Copy link
Member

bpasero commented Jan 7, 2016

@jrieken we do not support glob patterns for registering file names to languages currently. The code that decides which mode to load given a file name is https://github.com/Microsoft/vscode/blob/master/src/vs/editor/common/modes/languageExtensionPoint.ts#L451

@jrieken
Copy link
Member

jrieken commented Jan 7, 2016

closing in favour of #1851

@jrieken jrieken closed this as completed Jan 7, 2016
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

No branches or pull requests

5 participants