Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

fix(java-highlighting): disabling semantic highlighting #993

Merged
merged 1 commit into from
Feb 12, 2021

Conversation

sunix
Copy link
Contributor

@sunix sunix commented Feb 12, 2021

What does this PR do?

Fix syntax highlighting when semantic highlighting is available. Will reconsider re-enabling it once we fix this: eclipse-theia/theia#9058

Screenshot/screencast of this PR

Selection_221

What issues does this PR fix or reference?

PR alternative#2 for eclipse-che/che#18448

How to test this PR?

  1. Start a workspace with Java/maven devfile provided https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-993/simple/che-theia-maven-devfile.yaml
  2. Open a java file
  3. Wait for language server to be activated
  4. The syntax highlighting is OK

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Happy Path Channel

HAPPY_PATH_CHANNEL=stable

Signed-off-by: Sun Seng David TAN <sutan@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Feb 12, 2021

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:993
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:993

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@@ -10,6 +10,7 @@
"defaultIconTheme": "theia-file-icons",
"preferences": {
"editor.autoSave": "on",
"editor.semanticHighlighting.enabled": false,
Copy link
Member

Choose a reason for hiding this comment

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

Why disabling the semantic highlighting feature completely?
I quickly tried it and I've noticed the error from Java LS:

[Error - 8:43:11 PM] Feb 12, 2021, 8:43:11 PM Failed to resolve user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
java.net.URISyntaxException: Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
	at java.base/java.net.URI$Parser.fail(URI.java:2938)
	at java.base/java.net.URI$Parser.checkChars(URI.java:3109)
	at java.base/java.net.URI$Parser.parse(URI.java:3135)
	at java.base/java.net.URI.<init>(URI.java:623)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.toURI(JDTUtils.java:875)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.resolveCompilationUnit(JDTUtils.java:159)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceEventsHandler.didChangeWatchedFiles(WorkspaceEventsHandler.java:82)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.didChangeWatchedFiles(JDTLanguageServer.java:487)
	at jdk.internal.reflect.GeneratedMethodAccessor16.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:65)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.notify(GenericEndpoint.java:152)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleNotification(RemoteEndpoint.java:220)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:187)
	at org.eclipse.jdt.ls.core.internal.ParentProcessWatcher.lambda$0(ParentProcessWatcher.java:123)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
	at java.base/java.lang.Thread.run(Thread.java:832)

[Error - 8:43:11 PM] Feb 12, 2021, 8:43:11 PM Failed to resolve user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
java.net.URISyntaxException: Illegal character in scheme name at index 4: user_storage:/user/workspace-storage/79f41834b503f0268aa9d3a3d24b2ff2/redhat.java/jdt_ws/.metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs
	at java.base/java.net.URI$Parser.fail(URI.java:2938)
	at java.base/java.net.URI$Parser.checkChars(URI.java:3109)
	at java.base/java.net.URI$Parser.parse(URI.java:3135)
	at java.base/java.net.URI.<init>(URI.java:623)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.toURI(JDTUtils.java:875)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.findFile(JDTUtils.java:802)
	at org.eclipse.jdt.ls.core.internal.JDTUtils.getFileOrFolder(JDTUtils.java:968)
	at org.eclipse.jdt.ls.core.internal.managers.StandardProjectsManager.fileChanged(StandardProjectsManager.java:216)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceEventsHandler.didChangeWatchedFiles(WorkspaceEventsHandler.java:108)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.didChangeWatchedFiles(JDTLanguageServer.java:487)
	at jdk.internal.reflect.GeneratedMethodAccessor16.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.lambda$null$0(GenericEndpoint.java:65)
	at org.eclipse.lsp4j.jsonrpc.services.GenericEndpoint.notify(GenericEndpoint.java:152)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleNotification(RemoteEndpoint.java:220)
	at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:187)
	at org.eclipse.jdt.ls.core.internal.ParentProcessWatcher.lambda$0(ParentProcessWatcher.java:123)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
	at java.base/java.lang.Thread.run(Thread.java:832)


I get it each time when I'm changing Java's semantic highlighting preference:

jdt

Disabling the feature completely doesn't mean the bug is fixed. Right?
I believe it worth investing time into investigating the error from Java Plug-in and get the proper semantic highlighting for Java, but not hide the problem.

Copy link
Contributor Author

@sunix sunix Feb 12, 2021

Choose a reason for hiding this comment

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

Well it is fixing the bug from the user perspective: syntax highlighting is working as expected.

We agree with @ericwill that this would be a quickfix first and create a long term gh-issue for now.

So for semantic highlighting, investigation is happening there: eclipse-theia/theia#9058 if we think it is an important feature, should be prioritized accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, this is a quick fix to improve the UX and also not break upstream test cases. We will probably be upgrading the Java plugin next sprint, so we can investigate this issue alongside that.

@ericwill
Copy link
Contributor

[crw-ci-test]

Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

I'm approving it as a temporary quick fix to improve the UX.
The underlying Java LS error should be investigated and fixed in the subsequent sprints.

@sunix
Copy link
Contributor Author

sunix commented Feb 12, 2021

anyway @azatsarynnyy I don't think the java ls error is related. No error but the problem with semantic highlighting is still there:

image

@sunix sunix merged commit af8b33d into master Feb 12, 2021
@sunix sunix deleted the fix-java-hightlighting-che-theia branch February 12, 2021 20:47
@che-bot che-bot added this to the 7.27 milestone Feb 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants