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

Missing diagnostics messages #1509

Open
robb-j opened this issue Apr 9, 2023 · 7 comments
Open

Missing diagnostics messages #1509

robb-j opened this issue Apr 9, 2023 · 7 comments

Comments

@robb-j
Copy link

robb-j commented Apr 9, 2023

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

Screen.Recording.2023-04-09.at.21.48.43.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
Copy link
Author

robb-j commented Apr 9, 2023

Adding another log with the "initialise" messages in it

lsp.log

@angelozerr
Copy link
Contributor

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

@angelozerr
Copy link
Contributor

Is it working with vscode-xml?

@robb-j
Copy link
Author

robb-j commented Apr 11, 2023

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
Copy link
Contributor

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
Copy link
Author

robb-j commented Apr 11, 2023

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

if (range != null) {
length = changeEvent.getRangeLength().intValue();
} else {

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants