File association pattern validation

[AlexXuChen]

Related to #573.

When a grammar file is associated with an XML document using xml.fileAssociations, we should ensure that the pattern of the XML document file(s) (pattern field) is valid.

For example, when the file association pattern input is accepted:

And the text entry is an invalid pattern, no file association similar to the example below should be entered:

{
"pattern": "**/<invalid pattern>.xml",
"systemId": "<some grammar file path>
}

and instead there should be an error thrown with no file association.

[angelozerr]

@rgrunber here a demo with the current work:

As you can see in the demo, user can fill the pattern (**/foo2.xml) and if it changes to **/bar.xml, the pattern doesn't match the XML file to bind. This idea of this issue is to validate this file with a custom command on LemMinx side:

• add a validator on vscode-xml side at

  vscode-xml/src/commands/registerCommands.ts

  Line 219 in 5f56d3a

  prompt: "Enter the pattern of the XML document(s) to be bound."

• develop a custom command on LemMinx side like https://github.com/eclipse/lemminx/blob/master/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/commands/CheckBoundGrammarCommand.java which waits for a pattern and an URI (full file path). You could create a static method in https://github.com/eclipse/lemminx/blob/81298daf1d6aab4a5fd06fab3675affbf8fbff5d/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/PathPatternMatcher.java#L50 public static boolean matches(URI uri, String pattern)

[rgrunber]

I just have one general question about this approach. Initially I thought the reason for validation is to confirm that some file in the workspace folder(s) would match the file association pattern when the user is typing it in the wizard, so they don't waste time creating a pattern that matches nothing.

Now I see that the language server can only know about files that are opened, since we're not going to be sending every file Uri over for the validation. In fact our language server doesn't make use of the workspace folder setting from initialize and I think it would even break protocol to have it checking the filesystem.

So @angelozerr, you suggested to just send over the pattern & uri, and confirm that the pattern would match. Is there any reason we want to force this check to happen on the language server ? Even if other clients would be able to use it, it's a simple pattern check, that they could just as easily implement in their own client's wizard validation logic. If it were a more complicated validation, I would completely agree it should be on the language server side to save time, but I'm just curious about whether we draw any distinction on ease of implementation in deciding whether something gets done on the client, or server side.

[angelozerr]

I just have one general question about this approach. Initially I thought the reason for validation is to confirm that some file in the workspace folder(s) would match the file association pattern when the user is typing it in the wizard, so they don't waste time creating a pattern that matches nothing.

Sorry with my bad explanation. The idea is to write file association for the current file which is bound. We provide a default pattern which works but user can change it (to use * for instance inside file name to check another files that the current). When user change this pattern, the match should continue to work for the current file which is bound.

Now I see that the language server can only know about files that are opened, since we're not going to be sending every file Uri over for the validation. In fact our language server doesn't make use of the workspace folder setting from initialize and I think it would even break protocol to have it checking the filesystem.

Yes I think.

deciding whether something gets done on the client, or server side.

The main reason why I would like to this check on server side is that association with xsd/dtd with file association pattern is done on server side. So the check on wizard should be done on server side to share the same checking behavior (TypeScript glob pattern could have a different behavior from Java glob pattern for instance).

[rgrunber]

Ok, makes sense. I will make the necessary changes.

[rgrunber]

@angelozerr Just curious, why did I need to specify ClientCommandConstants.EXECUTE_WORKSPACE_COMMAND in https://github.com/redhat-developer/vscode-xml/blob/master/src/commands/registerCommands.ts#L223 here, but it works find without it here https://github.com/redhat-developer/vscode-xml/blob/master/src/commands/registerCommands.ts#L316 .

[angelozerr]

Just curious, why did I need to specify ClientCommandConstants.EXECUTE_WORKSPACE_COMMAND

It should work without it, no?

[rgrunber]

Just curious, why did I need to specify ClientCommandConstants.EXECUTE_WORKSPACE_COMMAND

It should work without it, no?

When I tried it, I kept getting message:'command 'xml.check.file.pattern' not found'. Adding the above fixed it, but I couldn't figure out why I needed to when other commands didn't.