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

Support the semantic highlighting based on the LSP extension proposal #715

Closed
kittaakos opened this issue Jul 2, 2018 · 9 comments
Closed

Comments

@kittaakos
Copy link

kittaakos commented Jul 2, 2018

I have submitted a proposal to extend the LSP with the semantic highlighting support.

I have already adjusted the LSP4J API based on my proposal. See also the vscode-languageserver-node implementation

Plans:

  • Adopt the Java LS to support the semantic highlightings. My plan is to copy over a bunch of org.eclipse.jdt.internal.ui.javaeditor.Semantic* classes from the JDT UI to the LS and get rid of the Eclipse Workbench, JFace, and SWT dependencies.
  • The goal is to support the LS based semantic highlighting in Eclipse Theia, VS Code, and Eclipse Che.

Suggestions and feedback are welcome. Thanks!

Edit: updated the plans.

kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 4, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 4, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@fbricon
Copy link
Contributor

fbricon commented Jul 4, 2018

@kittaakos be aware the Technology PMC is currently frowning very hard at jdt.ls forking jdt.ui classes over (see #684 (comment)). They'd strongly prefer to refactor jdt.ui code over to jdt.core, if doable.

@kittaakos
Copy link
Author

Thank you for the note, @fbricon.

CC: @svenefftinge

@svenefftinge
Copy link

@kittaakos be aware the Technology PMC is currently frowning very hard at jdt.ls forking jdt.ui classes over (see #684 (comment)). They'd strongly prefer to refactor jdt.ui code over to jdt.core, if doable.

So you think we should

  1. extract the code needed to from jdt.ui to jdt.core
  2. make use of those changes in jdt.ls

Sounds more sustainable but also like a lot of extra rounds :|

@fbricon
Copy link
Contributor

fbricon commented Jul 4, 2018

Sounds more sustainable but also like a lot of extra rounds :|

Tell me about it.
But we have @rgrunber and @jjohnstn to help if needed

@svenefftinge
Copy link

We could go in parallel. I.e.

  • do a PR here with the copied code
  • do a PR for jdt.core moving the code.

And then once jdt.ls upgrades to a version that has those changes we could remove the copied code here.

kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 5, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 5, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 5, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 10, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@rgrunber
Copy link
Contributor

For contributing back to JDT, given that the code you're copying comes from org.eclipse.jdt.ui, it would be easier to move it into org.eclipse.jdt.core.manipulation (resides in same git repo as org.eclipse.jdt.ui) as opposed to org.eclipse.jdt.core, which is a separate repository.

When copied into JDT-LS much of the functionality needed for the JDT UI could be removed but would need to be preserved if contributing upstream.

You could then just register an account at Eclipse and push your changes for review to ssh://username@git.eclipse.org:29418/jdt/eclipse.jdt.ui

kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 12, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@kittaakos
Copy link
Author

The following are the JDT UI dependencies I am currently resuing for this task:

We do not need:

  • The entire Eclipse Job support built around the semantic highlighting capability in JDT UI.
  • We do not need want the mutable nature of the Position instances plus the position updater, and semantic highlighting reconciliation.

kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 16, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 16, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Jul 18, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 6, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 6, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@kittaakos
Copy link
Author

A work in progress PR for the task: #746

@kittaakos
Copy link
Author

@rgrunber, @fbricon please take a look and give feedback. Thank you!

kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 7, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 27, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 27, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 27, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 30, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 30, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 30, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Aug 30, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Sep 13, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Sep 20, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit to kittaakos/eclipse.jdt.ls that referenced this issue Oct 12, 2018
Closes eclipse-jdtls#715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
fbricon pushed a commit that referenced this issue Oct 19, 2018
Closes #715.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants