Manage snippet registry to write snippet in JSON

[angelozerr]

Fixes #640

Signed-off-by: azerr [email protected]

[angelozerr]

This PR manages snippets with vscode JSON format. In other words

• existing snippets:

  • CDATA snippets
  • Comments snippets
  • DTD ELEMENT, ENTITY, ATTLIST snippets (inside DTD or DOCTYPE)

• new snippets for doctype.

Here some advanced snippet support:

• snippet are displayed according some condition (ex : DOCTYPE snippet is working only if file is empty or if completion is trigger before the root element which doesn't declare a DOCTPE.
• when DOCTYPE snippet is applied, it uses teh root element name to generate the proper DOCTYPE.

[angelozerr]

Here a demo:

[angelozerr]

@fbricon @dakwon you can play now with my PR, I need to clean my code and add some other tests.

[fbricon]

xml prolog snippet should be the 1st choice in an empty xml document. It's totally missing

[fbricon]

xml prolog should also probably be added to all snippets

[fbricon]

If there's an xml prolog already, the doctype snippets are not even proposed on the next line

[xorye]

I'm very excited for this PR.

For the prolog snippet named "Insert xml Processing Instruction with standalone", one of the placeholders are appearing:

Also, I think the space before the ?> at the end should be gone?

[angelozerr]

xml prolog should also probably be added to all snippets

done

If there's an xml prolog already, the doctype snippets are not even proposed on the next line

fixed

For the prolog snippet named "Insert xml Processing Instruction with standalone", one of the placeholders are appearing:

fixed

Also, I think the space before the ?> at the end should be gone?

fixed

@xorye could you play again with this PR and give me feedback, thanks!

[angelozerr]

xml prolog snippet should be the 1st choice in an empty xml document. It's totally missing

I have not managed sort for te moment, could we see that in an another PR please.

[angelozerr]

xml prolog should also probably be added to all snippets

It can be hard to do that because we need to manage case of existing or not of xml processing instruction to avoid generating it twice.

I had the problem with DOCTYPE (which generates a root-element if doens't exist and don't generate it otherwise). To do that I have duplicated snippets (see newfile-snippets.json and doctype-snippets.json).

To manage properly this usecase we should manage aggregation of several snippets, but perhaps we could do that in the future?

When I get completion after typing option?

I removed it.

Also, when I select Insert xml Processing Instruction or Insert xml Processing Instruction with standalone, I think there is a problem with the replace range:
There is an extra <?. I only get this problem with the prolog snippets.

@xorye fixed

Also if completion is invoked inside a comment like this:
I don't think we should be getting any snippets in this case

@xorye fixed

Can we also check for DocType? If I invoke completion at <!DOCTYPE |root-element [, I don't think I should get the prolog snippets.

@xorye fixed

Please fix the merge conflict

@fbricon fixed

The JSON snippet needs to be improved again, but if you are OK could we done that in separate issues:

• gives the capability to sort the snippets (xml prolog snippet should be the 1st choice in an empty xml document. )
• snippet should take care of quote preferences.

The new work that I have done is:

• eat the '>' when it exists. I mean when we have <? | > and xml processing instruction is applied, it eats the > and it generates

<?xml version="1.0" encoding="UTF-8"?>

• new snippets schemaLocation and noNamespaceSchemaLocation

[xorye]

Great, the fixes work on my machine.

I found a small inconsistency, XML should either always be upper case or always be lower case.

[xorye]

For the noNamespaceSchemaLocation snippet, the attributes are on a separate line:

<root-element
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:noNamespaceSchemaLocation="file.xsd">
    
</root-element>

A cool enhancement (maybe this can be done on a different issue) would be to have the attributes on a separate line if xml.format.splitAttributes is true.

Taking it one step further, it might be cool to have snippet text be preformatted against the formatter.
Because if the user has format on save enabled, and the user saves the document after inserting a snippet, I think it would be weird if the snippet content's formatting changes dramatically (like in the case for split attributes). But things like this could be discussed in another issue.

[angelozerr]

I found a small inconsistency, XML should either always be upper case or always be lower case.

fixed

[angelozerr]

But things like this could be discussed in another issue.

I agree with you we should pay attention with format settings in otherwise snippet should be formatted (each time or better one time and cache the formatted snippet).

Please create an issue for that. Thanks

[fbricon]

Jenkins fails https://ci.eclipse.org/lemminx/blue/organizations/jenkins/lemminx/detail/PR-680/34/pipeline:

[ERROR] Failures: 

[ERROR]   XMLCompletionSnippetsTest.afterComment:118 expected: <<!DOCTYPE root-element SYSTEM "file.dtd" >

<root-element>

</root-element>> but was: <<!DOCTYPE root-element SYSTEM "file.dtd">

<root-element>

</root-element>>

[ERROR]   XMLCompletionSnippetsTest.afterProlog:133 expected: <<!DOCTYPE root-element SYSTEM "file.dtd" >

<root-element>

</root-element>> but was: <<!DOCTYPE root-element SYSTEM "file.dtd">

<root-element>

</root-element>>

[ERROR]   XMLCompletionSnippetsTest.emptyXMLContent:38 expected: <<!DOCTYPE root-element SYSTEM "file.dtd" >

<root-element>

</root-element>> but was: <<!DOCTYPE root-element SYSTEM "file.dtd">

<root-element>

</root-element>>

[angelozerr]

@fbricon fixed

[xorye]

The snippet is eating more than the >:

If this is not a big problem, I'm good with this PR being merged

[angelozerr]

If this is not a big problem, I'm good with this PR being merged

No it's very bad -( Let me try to fix it.