Skip to content

feat: Adopt the Java Language Server's new API#1019

Merged
jdneo merged 1 commit intomasterfrom
cs/syntax-api
Jul 6, 2020
Merged

feat: Adopt the Java Language Server's new API#1019
jdneo merged 1 commit intomasterfrom
cs/syntax-api

Conversation

@jdneo
Copy link
Copy Markdown
Member

@jdneo jdneo commented Jun 22, 2020

demo

@jdneo jdneo added this to the 0.23.1 milestone Jun 22, 2020
Comment thread package.json Outdated
{
"command": "java.test.explorer.runAll",
"when": "view == testExplorer",
"when": "view == testExplorer && serverMode != LightWeight",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As we dicussed before, there's potential risk that the name of this context value, serverMode, conflicts with other extensions or vscode itself. And here it seems to depend on redhat.java to set this context value.

As it's available to fetch serverMode thru extension.api, I suggest to set context value in this extension, with some identifier like java-test:serverMode, just as GitLens is doing.
But the cons are:

  • other extensions also have to define duplicate context values.
  • the name is confusing, indicating that serverMode belongs to test runner (but not true).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Or rename the serverMode to java:serverMode in the upstream once the confliction occurs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jdneo jdneo marked this pull request as draft June 28, 2020 07:27
@jdneo jdneo marked this pull request as ready for review July 1, 2020 03:17
@jdneo jdneo merged commit 06e9628 into master Jul 6, 2020
@jdneo jdneo deleted the cs/syntax-api branch July 6, 2020 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants