Improve fileMatch patterns to allow more granular use

[ssbarnea]

Is your enhancement related to a problem? Please describe.

At this moment fileMatch pattern is suffering from several limitations that make it impossible to write patterns that correctly identify Ansible file types.

Describe the solution you would like

To see a full list o patterns that can reliably be used to identify Ansible file types, look at https://github.com/ansible-community/ansible-lint/blob/master/src/ansiblelint/config.py#L14-L33

There are two missing feature that prevent that:

• * expands as anything instead of current file. foo/*.yml currently also matching foo/bar/*.yml when instead it should not. The correct way to do it would be with foo/**/*.yml or foo/*/*.yml, where the second one requires one sublevel folder but the first one allows none, one or many subfolders feel (aka the recursive one).
• There is no support for {...}, so a pattern like "**/{host_vars,group_vars,vars,defaults}/**/*.{yaml,yml}"} must be exploded to 8 different patterns in order to work with current fileMatch. You can easily see how we can endup with very long and hard to maintain patterns.

Describe alternatives you have considered

While for the second missing feature we can probable live with the extra inconvenience, there is no solution for the first one. That is directly affecting Ansible because Ansible has a layout where nested patterns are needed and we must be able to control the matching to a single folder depth

Additional context

The given examples of patterns are based on https://facelessuser.github.io/wcmatch/ which is a python matching library but they are not unique to them. At least the globstar (**) is part of the extended glob syntax. Based on https://en.wikipedia.org/wiki/Glob_(programming)#cite_note-bashpat-10 it seems that there are at least two popular JavaScript implementation that should support it: minimatch (npm) and micromatch (babel/yarn).

[JPinkney]

I like the idea of switching to either minimatch or micromatch. I found a github gist of the differences and it seems that micromatch is slightly better. For reference the json language service is moving towards switching their file matching as well to fix issues like this

[joshuawilson]

I would also point out that minimatch is no longer being maintained. I would suggest using https://github.com/micromatch/micromatch

[ssbarnea]

I tried to make use of micromatch but I had some problems making use of it because is JS instead of TS and I do not yet master the tricks of making it importable without upsetting eslint.

[ssbarnea]

For reference: https://github.com/redhat-developer/yaml-language-server/compare/master...ssbarnea:micromatch?expand=1 -- imaybe someone knows how to correctly use micromatch in TypeScript as current code does not compile.

[evidolob]

@ssbarnea Change YAMLSchemaService is not enough as it extends JSONSchemaService which still use old style patterns.
To complitly finish #412 any way we need to write own implementation. As during JSON parsing we are losing offsets of JSON Schema tokens and I cannot implement navigation to particular part of JSON Schema which cause error as I have only JS object and doesn't know where in JSON Schema it placed.
I can start fixing this by replacing JSONSchemaService with our code in YAMLSchemaService and use micromatch for fileMatch patterns, and then return to #412
@JPinkney WDYT?

[ssbarnea]

I got some fresh feedback on microsoft/vscode-json-languageservice#80 (comment) so I guess there is a change to fix this at the source! If we do it before it gets cold again.

[JPinkney]

Ideally we can get the PR merged on vscode-json-languageservice and then just make sure our code works with it.

I can start fixing this by replacing JSONSchemaService with our code in YAMLSchemaService and use micromatch for fileMatch patterns, and then return to #412

Yeah, that makes sense. I haven't taken a look yet, would there be a lot of changes required?

[evidolob]

I haven't taken a look yet, would there be a lot of changes required?

No, but I don't think that it will requires a lot of changes.
Going to provide a draft PR with that.

[evidolob]

@ssbarnea @JPinkney @joshuawilson I create the PR with minimatch usage.
Could you look on it?

[maxullman]

Alternatively or additionally to blobs, it would be nice if regular expressions could be used to match file paths, and it was already implemented at one point d4a05e3

[ssbarnea]

Upstream already adopted minimatch and #448 is supposed to make it available to use.

I am not sure if further changes are needed or if we can consider this already done.

[JPinkney]

I think we might still might have to adapt https://github.com/redhat-developer/yaml-language-server/blob/master/src/languageservice/parser/isKubernetes.ts#L15 because it looks like it still uses the old file pattern association

[GerryFerdinandus]

@ssbarnea
I assume this issue is still open because you are not able to get your use case glob pattern scenario working?

[ssbarnea]

If I remember well microsoft already switched to using a more modern pattern matcher few months back. I was waiting for a yaml-language-server to catch-up and a new version to be released as part of vscode-ansible in order to update my patterns. Does the latest release include pattern matcher updates? If so I could give it another go.

[evidolob]

We use new matters with #448 and it was included in 0.19.0 release.
Now vscode-json-languageservice switched from minimatch to globregexp microsoft/vscode-json-languageservice@3c8b871 in lates release.
Should we also switch?

[ObserverOfTime]

@(mini|micro)match support extglobs which is precisely what I came here to request before finding this issue.

"yaml.schemas": {
    "github-issue-forms": ".github/ISSUE_TEMPLATE/!(config).yml",
    "github-issue-config": ".github/ISSUE_TEMPLATE/config.yml"
}

[ssbarnea]

I am still trying to find a negating pattern that works for making these exclusive (avoid having both active at the same time):

    "file:///Users/ssbarnea/c/a/schemas/f/ansible-tests-meta.json": [
      "tests/**/meta/main.yml"
    ],
    "file:///Users/ssbarnea/c/a/schemas/f/ansible-meta.json": [
      "**/meta/main.yml",
    ],

I tried various ways but not managed to negate the second one.

The full path would be like /Users/ssbarnea/c/a/schemas/test/tests/integration/role_foo/meta/main.yml and because we have tests before, I do not want to select the second schema.

I tried !(tests)**/meta/main.yml, !(tests/)**/meta/main.yml and it seems that it always picks both. I used Reload Window to ensure that it reloads it.

[ssbarnea]

Apparently internally we use regex and if we want to exclude specific text from being include a regex like ^((?!tests\/).)+\.yml should work.

That makes me believe that we could attempt to convert !(foo/) into ^((?!foo\/).)+ when creating the regex. The ^ is needed as we do not want to accidentally match as a substring.

Am I going into the right direction with this or we should better try to replace our implementation with the same one that vscode is using?

[ssbarnea]

Based on https://github.com/microsoft/vscode/blob/b35bc5e02109682f50ec2ff4ef6537c50ed75cb9/src/vs/workbench/api/common/jsonValidationExtensionPoint.ts#L29 they officially support ! as a marker for making a pattern negative.

I am inclined to say that we should do the same, even before switching to the same implementation. If glob pattern starts with ! it should be transformed into a negative match.

@evidolob @msivasubramaniaan WDYT? Should I try to patch our implementation to do it?

[jeremyfiel]

@ssbarnea any update on this? also running into this issue.