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.

Sign up for GitHub

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

Jump to bottom

refine semantic highlighting #1416

Merged
merged 4 commits into from
Apr 27, 2020
Merged

refine semantic highlighting #1416

merged 4 commits into from
Apr 27, 2020

Conversation

Eskibear
Copy link
Contributor

@Eskibear Eskibear commented Apr 23, 2020 •
edited
Loading

References:
Semantic-Highlighting-Overview#token-classification
specs for Semantic Highlighting proposed in LSP 3.16

Change log for this PR

Fixed

  • wrong range for multiple tokens in one line

Added

  • semantic highlight for more token types
    • types (highlighted as type)
    • packages (highlighted as namespace)
    • member variables (highlighted as property)

Changed

  • following LSP proposal to use standard modifier readonly instead of final

@Eskibear
Copy link
Contributor Author

Eskibear commented Apr 23, 2020 •
edited
Loading

It's more straightforward to show visualized effect than request/response. Below are some legacy issues that could not be covered by syntax highlighting.

Left: behavior after this PR, Right: previous
(no extra settings, all render by vscode)

redhat-developer/vscode-java#299
image

redhat-developer/vscode-java#338
image

redhat-developer/vscode-java#351
image

redhat-developer/vscode-java#707
image

redhat-developer/vscode-java#826
image

redhat-developer/vscode-java#1277
image

@Eskibear Eskibear changed the title [WIP] refine semantic highlighting refine semantic highlighting Apr 26, 2020
@Eskibear
Copy link
Contributor Author

Eskibear commented Apr 26, 2020 •
edited
Loading

It depends on #1412 and #1418 for robustness improvement. If you want to try it by yourself, please also include changes in them.

UPDATE:
rebased, as 1412 and 1418 have been merged.

@Eskibear
refine test cases for adding more token types
de60268 
Signed-off-by: Yan Zhang <[email protected]>
@Eskibear
highlight type, constructor and package
b1cc5cb 
Signed-off-by: Yan Zhang <[email protected]>

update test cases

Signed-off-by: Yan Zhang <[email protected]>
@Eskibear
diff field/local variables
43614e4 
Signed-off-by: Yan Zhang <[email protected]>
fbricon
fbricon requested changes Apr 26, 2020
View reviewed changes
Copy link
Contributor

@fbricon fbricon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This dramatically improves semantic highlighting. My main gripe about it is about the delay one sees between switching from one highlighting mode to another, when opening the file. Also highlighting is sometimes delayed as you type, but I'm not sure this is fixable, given that we need to wait for all CUs to be reconciled as-you-type

...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/semantictokens/SemanticTokensVisitor.java Show resolved Hide resolved
@Eskibear
Copy link
Contributor Author

about the delay one sees between switching from one highlighting mode to another

FYI, there's also some delay for TS (but I feel it's shorter compared with Java), also mentioned in Semantic Highlighting Overview/FAQ.

I'm not sure this is fixable, given that we need to wait for all CUs to be reconciled as-you-type

Agreed, it looks not feasible for the moment.

@Eskibear
simplify code with stream
ac02ca2 
Signed-off-by: Yan Zhang <[email protected]>
@fbricon fbricon merged commit 82b9b21 into eclipse-jdtls:master Apr 27, 2020
@fbricon
Copy link
Contributor

fbricon commented Apr 27, 2020

@Eskibear thanks for these improvements!

@fbricon fbricon added this to the End April 2020 milestone Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbricon fbricon fbricon requested changes

Assignees
No one assigned
Labels
enhancement semantic highlighting
Projects
None yet
Milestone
End April 2020
Development

Successfully merging this pull request may close these issues.
2 participants
@Eskibear @fbricon