lemminx icon indicating copy to clipboard operation
lemminx copied to clipboard

Missing diagnostics messages

Open robb-j opened this issue 2 years ago • 7 comments

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 avatar Apr 09 '23 21:04 robb-j

Adding another log with the "initialise" messages in it

lsp.log

robb-j avatar Apr 09 '23 21:04 robb-j

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

angelozerr avatar Apr 10 '23 00:04 angelozerr

Is it working with vscode-xml?

angelozerr avatar Apr 10 '23 00:04 angelozerr

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.

robb-j avatar Apr 11 '23 12:04 robb-j

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.

angelozerr avatar Apr 11 '23 13:04 angelozerr

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?

robb-j avatar Apr 11 '23 16:04 robb-j

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.

angelozerr avatar Apr 11 '23 18:04 angelozerr