Missing diagnostics messages

[robb-j]

Hi, I've been working on an XML Extension that uses this Language Server and have encountered an issue.

I only seem to get an initial textDocument/publishDiagnostics when opening a document and no following ones after the document is edited. I've been using the binary released with redhat-developer/vscode-xml, currently using their 0.24.0 build, I tried a few other versions but the same issue occurs.

I've got the LSP log from Nova and a screencast of what I was doing while it was captured:

lsp.log

https://user-images.githubusercontent.com/1526174/230796358-11d05a6b-51b7-4a23-9122-dc05fed53e34.mov

I'm not sure where to start debugging this, I was expecting a textDocument/publishDiagnostics notification after the textDocument/didChange to clear the error message? Any help/guidance would be appreciated.

[robb-j]

Adding another log with the "initialise" messages in it

lsp.log

[angelozerr]

Hvae you this problem with another file? Is it possible to share your XML and XSD please.

[angelozerr]

Is it working with vscode-xml?

[robb-j]

It seems to be for any file not just that specific one. The example above was the examples/KitchenSink.xml from my extension repo. The same issue happens with Inventory.xml and Inventory.xsd in that same folder which is a simpler reproduction.

Both files validate fine in vscode-xml and the diagnostics go away once fixed.

[angelozerr]

Both files validate fine in vscode-xml and the diagnostics go away once fixed.

If I understand your comment, it is working on vscode-xml? If it that it is a problem with nova LSP client.

[robb-j]

Yes, something is up, either in Nova or Lemminx. It seems the server isn't sending a textDocument/publishDiagnostics message after a textDocument/didChange is sent to it, so the diagnostic errors don't go away.

I got a debugging version running on my local machine and there is a NullPointerException being thrown from TextDocumentContentChangeEvent#getRangeLength because Nova is not setting the "rangeLength" . From the spec that is allowed?

This is the code that is calling it https://github.com/eclipse/lemminx/blob/b34a847c51043ccf782da8646d25323a1fa290ab/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/commons/TextDocument.java#L169-L171

Here's the error log: lsp4xml.log

If you can point me in the right direction I'm more than happy to submit a PR if it is an issue with the server, or I can take this to Nova and submit a report there if it isn't being spec-compliant?

[angelozerr]

Good catch! Indeed lemminx consider that we have already a get range length (vscode and Eclipse IDE provides them).

If you can point me in the right direction I'm more than happy to submit a PR if it is an issue with the server

Indedd it is an issue with the server which should allow not to define rangeLengh.

I'm more than happy to submit a PR if it is an issue with the server,

Great!

I have not tested but the code should looks like this, just to give you some idea:

// Loop for each changes and update the buffer
for (int i = 0; i < changes.size(); i++) {

    TextDocumentContentChangeEvent changeEvent = changes.get(i);
    Range range = changeEvent.getRange();
    int length = 0;
    int startOffset = -1;
    if (range != null) {
        if (changeEvent.getRangeLength() != null) {
            // rangeLength is defined, use it
            length = changeEvent.getRangeLength().intValue();
        } else {
            // rangeLength is not defined, compute it
            startOffset = offsetAt(range.getStart());
            int endOffset = offsetAt(range.getEnd());
            length = endOffset - startOffset;
        }
    } else {
        // range is optional and if not given, the whole file content is replaced
        length = buffer.length();
        range = new Range(positionAt(0), positionAt(length));
    }
    String text = changeEvent.getText();
    startOffset = startOffset != 1 ? startOffset : offsetAt(range.getStart());
    buffer.replace(startOffset, startOffset + length, text);
    lineTracker.replace(startOffset, length, text);
}

The bad news is that we have no test with this update method, we should write them.

Hope it will help you.