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

bump LSP4J version 0.14.0 to 0.20.1 #1498

Merged
merged 5 commits into from
Mar 6, 2023

Conversation

w6et
Copy link
Contributor

@w6et w6et commented Mar 2, 2023

bump junit version 5.9.1 to 5.9.2
bump gson version 2.10 to 2.10.1
remove org.eclipse.xtend.lib from dependencyManagement(Transitive by LSP4J)

bump junit version 5.9.1 to 5.9.2
bump gson version 2.10 to 2.10.1
remove org.eclipse.xtend.lib from dependencyManagement(Transitive by LSP4J)
@@ -12,7 +12,7 @@
*/
package org.eclipse.lemminx;

import static org.eclipse.lsp4j.jsonrpc.CompletableFutures.computeAsync;
import static org.eclipse.lsp4j.jsonrpc.CompletableFutures.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use * for import.

}else {
return null;
}
//return getXMLLanguageService().prepareRename3(xmlDocument, params.getPosition(), cancelChecker);//comment at 2023/3/2 for update lsp4j 0.20.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this comment.

pom.xml Outdated
@@ -8,8 +8,10 @@
<description>LemMinX is a XML Language Server Protocol (LSP), and can be used with any editor that supports LSP, to offer an outstanding XML editing experience</description>
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<lsp4j.version>0.14.0</lsp4j.version>
<junit.version>5.9.1</junit.version>
<!--<lsp4j.version>0.14.0</lsp4j.version>-->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this comment

pom.xml Outdated Show resolved Hide resolved
@angelozerr
Copy link
Contributor

For binary support, please define new class PrepareRenameDefaultBehavior at

@w6et do you think there are another new classes? If yes please add it in the reflect-config.json file

add PrepareRenameDefaultBehavior to reflect-config.json
change XMLTextDocumentService#prepareRename return new PrepareRenameDefaultBehavior() if either is null
@angelozerr
Copy link
Contributor

I wonder if we should update too the Java 8 configuration:

<plugin>
					<groupId>org.apache.maven.plugins</groupId>
					<artifactId>maven-compiler-plugin</artifactId>
					<version>3.10.1</version>
					<configuration>
						<source>1.8</source>
						<target>1.8</target>
					</configuration>
				</plugin>

because I think LSP4J uses Java 7 now?

Otherwise it loogs good for me.

@datho7561 what do you think about this PR?

@datho7561
Copy link
Contributor

@w6et do you think there are another new classes? If yes please add it in the reflect-config.json file

As long as the binary is still working, we shouldn't need to add anything there.

because I think LSP4J uses Java 7 now?

It uses Java 8 / Java 1.8: https://github.com/eclipse/lsp4j/blob/main/org.eclipse.lsp4j/build.gradle#L16

@w6et
Copy link
Contributor Author

w6et commented Mar 3, 2023

I wonder if we should update too the Java 8 configuration:

/lemminx-parent/pom.xml has been specified java8 in pluginManagement ago,so don't need do this anymore

pom.xml Outdated Show resolved Hide resolved
@datho7561
Copy link
Contributor

Here are the changes you need to make to reflect-config.json so that the binary build works:

diff --git a/org.eclipse.lemminx/src/main/resources/META-INF/native-image/reflect-config.json b/org.eclipse.lemminx/src/main/resources/META-INF/native-image/reflect-config.json
index cb51dc57..0746f6c3 100644
--- a/org.eclipse.lemminx/src/main/resources/META-INF/native-image/reflect-config.json
+++ b/org.eclipse.lemminx/src/main/resources/META-INF/native-image/reflect-config.json
@@ -1525,6 +1525,22 @@
 			"name": "value"
 		}]
 	},
+	{
+		"name": "org.eclipse.lsp4j.NotebookDocumentClientCapabilities",
+		"allDeclaredFields": true,
+		"methods": [{
+			"name": "<init>",
+			"parameterTypes": []
+		}]
+	},
+	{
+		"name": "org.eclipse.lsp4j.NotebookDocumentSyncClientCapabilities",
+		"allDeclaredFields": true,
+		"methods": [{
+			"name": "<init>",
+			"parameterTypes": []
+		}]
+	},
 	{
 		"name": "org.eclipse.lsp4j.OnTypeFormattingCapabilities",
 		"allDeclaredFields": true,
@@ -2095,6 +2111,13 @@
 			"parameterTypes": []
 		}]
 	},
+	{
+		"name": "org.eclipse.lsp4j.adapters.DocumentDiagnosticReportTypeAdapter",
+		"methods": [{
+			"name": "<init>",
+			"parameterTypes": []
+		}]
+	},
 	{
 		"name": "org.eclipse.lsp4j.adapters.DocumentSymbolResponseAdapter",
 		"methods": [{

…cumentSyncClientCapabilities\DocumentDiagnosticReportTypeAdapter for native build
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks so much!

@datho7561 datho7561 merged commit d764495 into eclipse:main Mar 6, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants