Support for TransportKind.pipe through java.transport setting

[rgrunber]

Fixes #3282

[testforstephen]

Fall back to stdio in the default branch?

[rgrunber]

I think I noticed that leaving transport empty defaulted to stdio (ie. we haven't defined it until now and we're clearly doing stdio) but I guess we can be explicit now. I'll set the default to stdio in the setting for now.

[testforstephen]

The default value of the setting has a significant impact on the user experience, as most users are unlikely to customize it manually. Our ultimate goal is to make pipe the default value instead of stdio in the setting. However, if you are concerned about the potential issues of switching to pipe right away, we can introduce a new default value auto for the java.transport setting. At the moment, we can map auto to pipe for the vscode insider users (similar to how we handle android support below). This will allow us to use the insider users as testers for the new pipe support. If no issues arise for insider, we can then consider rolling out to all users later.

vscode-java/src/utils.ts

Lines 194 to 199 in 9e20e13

	const isInsider: boolean = version.includes("insider");
	const androidSupport = javaConfig.jdt.ls.androidSupport.enabled;
	switch (androidSupport) {
	case "auto":
	javaConfig.jdt.ls.androidSupport.enabled = isInsider;
	break;

[rgrunber]

Given that we agree to leave the setting out of package.json, auto wouldn't make sense as users would never really get to set it. I've configured the "insider" case to trigger in the default case of the switch. In other words, if java.transport is not set, and it is an insiders release, then we switch to pipe.

[testforstephen]

Another implementation is to check if the extension version is pre-release version. This can cover VS Code stable and insider better.

const javaExtVersion = javaExt.packageJSON?.version;
const isPreReleaseVersion = /^\d+\.\d+\.\d{10}/.test(javaExtVersion);

[rgrunber]

Nice, i'll make this change!

[rgrunber]

I think context.extension.packageJSON should be safe. I see it done as https://github.com/redhat-developer/vscode-java/blob/master/src/jdkUtils.ts#L12-L13 , but I would think accessing packageJSON directly is simpler. Maybe certain things are not included in the vscode parsed packageJSON so the file read approach is needed there.

[snjeza]

@rgrunber You would need to add java.transport to package.json.

[rgrunber]

@rgrunber You would need to add java.transport to package.json.

I've removed it based on #3355 (comment) . We want to avoid having it as an exposed setting for users, as the transport is an internal mechanism. We just want a way to test it in insiders/pre-release first and maybe allow more advanced users to set it.

[snjeza]

I've removed it based on #3355 (comment) . We want to avoid having it as an exposed setting for users, as the transport is an internal mechanism. We just want a way to test it in insiders/pre-release first and maybe allow more advanced users to set it.

What about -Djava.transport=pipe ? We would avoid Unknown Configuration Setting in settings.json.

[rgrunber]

I've removed it based on #3355 (comment) . We want to avoid having it as an exposed setting for users, as the transport is an internal mechanism. We just want a way to test it in insiders/pre-release first and maybe allow more advanced users to set it.

What about -Djava.transport=pipe ? We would avoid Unknown Configuration Setting in settings.json.

@testforstephen @fbricon , let me know your thoughts on this. I don't mind making it a system property.

[fbricon]

pseudo setting is fine by me. "Unknown Configuration Setting" is expected. This is a temporary measure anyway.