Adjust range for XML syntax errors about lemminx HOT 9 CLOSED

@angelozerr The XMLLocator does not have any chance to get a correct position for certain cases since the error can be determined after the locator has passed the token it needs the location information for.

Did you have an idea on how to handle this?

A possible solution is to use the XMLDocument and guess the node from the values we have.

from lemminx.

Indeed @NikolasKomonen see https://github.com/angelozerr/lsp4xml/blob/master/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/contentmodel/participants/diagnostics/XMLSyntaxErrorCode.java#L83 how I have started to fix it. It avoids parsing XMLDocument but perhaps it's a bad idea?

from lemminx.

A possible solution is to use the XMLDocument and guess the node from the values we have.

I have started to experiment your idea. If it's working, I will commit my work. Please wait before continue this issue.

from lemminx.

@angelozerr Alright, I have the previous implementation done but there are some minor problems with it. Hopefully your experimenting works.

from lemminx.

@NikolasKomonen you can now continue to work on this issue. I have cleaned XML diagnostics test (you will see that it's more readable).

Now I'm based on XMLDocument to adjust errors. Tell me if you like it and I think it improves the adjust.

from lemminx.

@NikolasKomonen just to say you, that I'm using https://github.com/kingargyle/wtp-sourceediting/blob/008173496f087062264ff4c77da2e7fdc36340c9/plugins/org.eclipse.wst.xml.core/src-validation/org/eclipse/wst/xml/core/internal/validation/eclipse/XMLMessageInfoHelper.java to know which selection I must do according the error code (select attribute value, name, etc)

from lemminx.

@NikolasKomonen once you have implemented an adjust error code, it should be cool that you have the same sample that you are using in JUNit in a given file like https://github.com/angelozerr/lsp4xml/tree/master/org.eclipse.lsp4xml/src/test/resources/validation/syntax

It will help us in the future to see the result in the editor (to avoid searching again a sample)

from lemminx.

@angelozerr Do you have a recommendation on how you would want to enable multiple error ranges to be sent?

For certain cases like ETagRequired and ETagUnterminated we need to have multiple ranges. I think that it would be better to have toLSPRange return an array of Ranges instead.

from lemminx.

For certain cases like ETagRequired and ETagUnterminated we need to have multiple ranges. I think that it would be better to have toLSPRange return an array of Ranges instead.

Yes we should do that and create a Diagnostic instances per range, because LSP Diagnostic has just one range.

from lemminx.