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

semantic tokens: use 'method' instead of 'member' #1713

Closed
aeschli opened this issue Nov 23, 2020 · 7 comments · Fixed by eclipse-jdtls/eclipse.jdt.ls#1608
Closed

semantic tokens: use 'method' instead of 'member' #1713

aeschli opened this issue Nov 23, 2020 · 7 comments · Fixed by eclipse-jdtls/eclipse.jdt.ls#1608

Comments

@aeschli
Copy link
Collaborator

aeschli commented Nov 23, 2020

After requests in microsoft/language-server-protocol#1087 we decided to deprecate the 'member' token type in favor of a new 'method' token type.

The 'method' token type stands for all types of member functions (also if the language calls it differently)

@aeschli
Copy link
Collaborator Author

aeschli commented Nov 23, 2020

VSCode issue: microsoft/vscode#111172

@0dinD
Copy link
Contributor

0dinD commented Nov 23, 2020

The semantic token for methods was never even renamed to member (from function), since I was waiting for your response to microsoft/language-server-protocol#1087. But now that everything is cleared up, I think it's time for a rename.

@0dinD
Copy link
Contributor

0dinD commented Nov 23, 2020

BTW, merging eclipse-jdtls/eclipse.jdt.ls#1608 will break styling of method tokens (unless explicitly styled) until microsoft/vscode@07d680b makes it into a release, since they currently are not mapped to any TextMate scope. This could be fixed by contributing more scope mappings like in #1636, for method tokens. But I'm not sure if it's a good idea since the scope mapping will eventually be provided by microsoft/vscode@07d680b. We could wait until we can specify the version dependency in the package.json of vscode-java, but I'm not sure if it matters for a small syntax highlighting fix like this. I'll let whoever reviews/merges eclipse-jdtls/eclipse.jdt.ls#1608 decide which route to take.

@aeschli
Copy link
Collaborator Author

aeschli commented Nov 24, 2020

Yes, you better wait until 1.52 is out (or restrict the extension to 1.52)

@fbricon
Copy link
Collaborator

fbricon commented Jan 25, 2021

@testforstephen what's the status on this one?

@testforstephen
Copy link
Collaborator

Since 1.52 is out for a while, we can move forward with pull request eclipse-jdtls/eclipse.jdt.ls#1608 to address this issue.

@testforstephen
Copy link
Collaborator

thanks @0dinD, this has been fixed by eclipse-jdtls/eclipse.jdt.ls#1608

@rgrunber rgrunber added this to the Early February 2021 milestone Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants