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

In Maven <configuration>, all known XML elements from schema are suggested as completion #612

Closed
mickaelistria opened this issue Mar 4, 2020 · 9 comments · Fixed by #620
Assignees
Labels
bug Something isn't working completion This issue or enhancement is related to completion support
Milestone

Comments

@mickaelistria
Copy link
Contributor

In Maven , all known XML elements from schema are suggested as completion.
This is typically not desired as the Maven extension can provide higher quality proposals here and the extra items are useless and generate noise reducing the usability.

@angelozerr
Copy link
Contributor

This behaviour is because of use of processContents in XML Schema of configuration:

<xs:element minOccurs="0" name="configuration">

        <xs:complexType>
          <xs:sequence>
            <xs:any minOccurs="0" maxOccurs="unbounded" processContents="skip"/>
          </xs:sequence>
        </xs:complexType>
</xs:element>

This is typically not desired as the Maven extension can provide higher quality proposals here and the extra items are useless and generate noise reducing the usability.

If I understand your need, you want to manage completion for configuration in your maven lsp4xml extension? If it that I agree and we should give the capability to override the default behaviour of XML Schema completion.

@mickaelistria
Copy link
Contributor Author

If I understand your need, you want to manage completion for configuration in your maven lsp4xml extension?

Yes

If it that I agree and we should give the capability to override the default behaviour of XML Schema completion.

It seems to me that processContents="skip" and the absence of target namespace means that the XML Schema completion/validation is to be totally ignored in that block (as opposed to strict or lax values).
It seems to me that the schema is correct and there are already enough info in it to decide of not adding completion items, without overriding anything.

@angelozerr
Copy link
Contributor

It seems to me that processContents="skip" and the absence of target namespace means that the XML Schema completion/validation is to be totally ignored in that block (as opposed to strict or lax values).

For validation yes (you can add any XML content), but for completion we decide to provide something for this issue
redhat-developer/vscode-xml#177 (comment)

In other words, some user want to have this completion behaviour, but I agree with you, if you write an extension and you wish to override this behaviour, it's annoying. So we need to give the capability for an extension to override this default XML Schema behaviour.

@mickaelistria
Copy link
Contributor Author

For validation yes (you can add any XML content), but for completion we decide to provide something for this issue redhat-developer/vscode-xml#177 (comment)

You mentioned on that comment that user should use lax and not skip, so aren't we agreeing here?
IMO, validation and completion are similar: if schema says it can validate, it means it can complete; if it says it skips validation, then it should skip complete.

In other words, some user want to have this completion behaviour

Why aren't they using lax then?

So we need to give the capability for an extension to override this default XML Schema behaviour.

It seems to me that the schema has everything necessary, the LS shouldn't facilitate the work with bad schemas if as a consequence it reduces the quality of the work with good schemas (like Maven one).

@angelozerr
Copy link
Contributor

After playing with WTP XML editor, it seems it shows root elements like project:

image

@mickaelistria
Copy link
Contributor Author

WTP XML isn't always a good reference ;) But if LemMinX is tweaked so that it would only show the "project" element as additional items in configuration block, it would already be nice and probably enough.

@angelozerr
Copy link
Contributor

WTP XML isn't always a good reference ;)

To be honnest with you, I have not a strong opinion about the proper behavior about skip.

@fbricon what do you think about that?

@fbricon
Copy link
Contributor

fbricon commented Mar 9, 2020

let's compare with other editors first

@angelozerr
Copy link
Contributor

It seems oxygen works like WTP XML Editor:

image

@angelozerr angelozerr self-assigned this Mar 9, 2020
@angelozerr angelozerr added bug Something isn't working completion This issue or enhancement is related to completion support labels Mar 9, 2020
@angelozerr angelozerr added this to the 0.11.0 milestone Mar 9, 2020
angelozerr added a commit to angelozerr/lemminx that referenced this issue Mar 12, 2020
fbricon pushed a commit that referenced this issue Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completion This issue or enhancement is related to completion support
Projects
None yet
3 participants