-
Notifications
You must be signed in to change notification settings - Fork 408
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
Provide preference mapping for Java Execution Environments -> Runtimes #1309
Conversation
So on a Mac, JDT automatically picks up JDKs under /Library/Java/JavaVirtualMachines/. If I set my custom JavaSE-11 pointing to /Users/fbricon/.sdkman/candidates/java/11.0.5.hs-adpt, when a project is configured to use JavaSE-11, it points to the JDK from /Library/Java/JavaVirtualMachines/jdk-11.jdk/. Since we explicitly set the preference, it should use that |
We can set more runtimes. I will add the default property, as follows:
|
what does default mean? default JavaSE-11 runtime? It should be unique anyways, from a user standpoint. Or default to the workbench? And what happens when someone copies/pastes another runtime and you end up with multiple defaults? |
only first will be set as default. The following is schema:
|
I agree we need a default jdk (eg. to use with invisible projects), but this doesn't change the fact that any JDK I set in the preference should be used by default vs anything picked up by JDT automatically |
|
No I want JavaSE-11 to be the default JavaSE-11 environment. The names themselves should be limited by the list of supported execution environment in eclipse. That's what I meant in https://github.com/redhat-developer/vscode-java/pull/1207/files#r360133208 |
test this please |
a9d6b34
to
1abdf44
Compare
@fbricon I have updated the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When saving the same runtime settings multiple times (doing a dummy or unrelated change in vscode's settings.json for instance), the list of runtimes is updated even if nothing changed. https://github.com/eclipse/eclipse.jdt.ls/pull/1309/files#diff-99e23b70fb2b30eae67e36c0455fd82cR148 should be false
} | ||
} | ||
} | ||
if (changed) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JavaLanguageServerPlugin.logInfo("JVM Runtimes changed, saving new configuration");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
if (compatibleVM.equals(vm)) { | ||
if (!environment.isStrictlyCompatible(vm)) { | ||
JavaLanguageServerPlugin.logInfo("Runtime at '" + vm.getInstallLocation().toString() + "' is not strictly compatible with the '" + name + "' environment"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JavaLanguageServerPlugin.logInfo("Setting " + compatibleVM.getInstallLocation() + " as " + name + "' environment (id:" + compatibleVM.getId() + ")");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
JavaLanguageServerPlugin.logInfo("Invalid runtime: " + runtime); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should log which runtime is the default VM (as multiple ones can have default:true, the last one wins)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
if (runtime.isValid()) { | ||
runtimes.add(runtime); | ||
} else { | ||
JavaLanguageServerPlugin.logInfo("Multiple runtime name:" + runtime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runtime is not valid:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
JavaLanguageServerPlugin.logInfo("Multiple runtime name:" + runtime); | ||
} | ||
} else { | ||
JavaLanguageServerPlugin.logInfo("Multiple runtime name:" + runtime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple runtimes with name:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@fbricon I have updated the PR. |
IPath systemSourcePath = sourcePath != null ? sourcePath : lib.getSystemLibrarySourcePath(); | ||
URL javadocLocation = javadocURL != null ? javadocURL : lib.getJavadocLocation(); | ||
LibraryLocation newLib = new LibraryLocation(lib.getSystemLibraryPath(), systemSourcePath, lib.getPackageRootPath(), javadocLocation, lib.getIndexLocation(), lib.getExternalAnnotationsPath()); | ||
libChanged = !newLib.equals(lib); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libChanged = libChanged || !newLib.equals(lib);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Once again, if the runtimes didn't change, they're still reset on every didchangeconfiguration request:
|
@fbricon I have updated the PR. |
test this please |
Related issue - #1317 |
Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
Thanks @snjeza! |
Fixes #1307
Requires redhat-developer/vscode-java#1207
Signed-off-by: Snjezana Peco snjezana.peco@redhat.com