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

conform spec: shutdown response does not conform to language-server spec #1508

Closed
KMikeeU opened this issue Apr 6, 2023 · 2 comments · Fixed by #1512
Closed

conform spec: shutdown response does not conform to language-server spec #1508

KMikeeU opened this issue Apr 6, 2023 · 2 comments · Fixed by #1512
Assignees
Labels
bug Something isn't working
Milestone

Comments

@KMikeeU
Copy link

KMikeeU commented Apr 6, 2023

From what I've found, the language-server spec specifies, that the response to a shutdown request should have a result of null (see here), however it seems that in XMLLanguageServer.java a new Object() is returned instead.

I wanted to create a pull request, but since that involved giving you my full name and contact information, I thought I would just add the suggested patch here, as it is very small.

From 5af36dcc72ae4e4eb03a86909dab28eed118e795 Mon Sep 17 00:00:00 2001
From: Michael <20937441+KMikeeU@users.noreply.github.com>
Date: Thu, 6 Apr 2023 23:47:02 +0200
Subject: [PATCH 1/1] spec: shutdown returns null

---
 .../src/main/java/org/eclipse/lemminx/XMLLanguageServer.java    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java
index baa11711..506138cf 100644
--- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java
+++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java
@@ -238,7 +238,7 @@ public class XMLLanguageServer implements ProcessLanguageServer, XMLLanguageServ
 		if (capabilityManager.getClientCapabilities().shouldLanguageServerExitOnShutdown()) {
 			delayer.schedule(() -> exit(0), 1, TimeUnit.SECONDS);
 		}
-		return computeAsync(cc -> new Object());
+		return computeAsync(cc -> null);
 	}

 	@Override
--
2.40.0
@datho7561 datho7561 added the bug Something isn't working label Apr 14, 2023
@datho7561 datho7561 self-assigned this Apr 14, 2023
@datho7561 datho7561 added this to the 0.25.0 milestone Apr 14, 2023
@datho7561
Copy link
Contributor

datho7561 commented Apr 14, 2023

Does this affect any clients that you've used? I tested it with VS Code and neovim, and either {} or null seems to work there.

I'll make the fix; we can also get rid of the computeAsync, since it makes no sense to cancel the shutdown request. This might save a few bytes of RAM during shutdown :D

datho7561 added a commit to datho7561/lemminx that referenced this issue Apr 14, 2023
Fixes eclipse-lemminx#1508

Signed-off-by: David Thompson <davthomp@redhat.com>
datho7561 added a commit to datho7561/lemminx that referenced this issue Apr 14, 2023
Fixes eclipse-lemminx#1508

Signed-off-by: David Thompson <davthomp@redhat.com>
@KMikeeU
Copy link
Author

KMikeeU commented Apr 14, 2023

Does this affect any clients

Yes I was planning to integrate with https://github.com/helix-editor/helix, the internal LSP just panics

Thank you!

fbricon pushed a commit that referenced this issue Apr 15, 2023
Fixes #1508

Signed-off-by: David Thompson <davthomp@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants